Repository: getsentry/responses
Pull request #416 (2021-10-21 02:28:44)

mbeliaev/registry
I went through #411 and, sincerely, I had absolutely the same need and was thinking how would be better to implement it.
thanks to @carloshorn, who inspired with the idea of Registry

I propose fully backwards compatible implementation.
Short description:
1. User is provided with `registries.BaseRegistry` as abstract class that must be implemented
2. in custom class use can define any logic of finding match (user just redefines `find` function that must return `found_match, match_failed_reasons`), eg examples mentioned in #411 
  * strict: provide every response only once.
  * very strict: only provide the next response.
  * insertion: always provide responses in insertion order.
  * rotating: move found matches to the end, i.e. if they are already in order it performs a rotation (might play well with the replace method if the other responses stay the same.)
  * fall-back: the current selection order  
3. then user can replace registry for default mock by running `responses.set_registry(CustomRegistry)` or just set as argument in `RequestsMock`

idea is the same as with `matchers`. Once we shift to `registries`, then we can populate them with some predefined, eg like in #411 or some from point 2 above. Then, users will have easy access to new types of response registries out of the box with single line of code

Note:
once we agreed to proceed with implementation, then I will polish this PR: docs, test, typing, changelog

carloshorn: I had to implement my own version of this feature to understand your design choices regarding the setter and getter for the registry. So you need the setter method for interaction with the module level default instance of `RequestsMock`, which you achieve by exporting the setter function.

I agree with @markstory that *custom registries is only for niche/power user usage*. So I implemented the registry as normal attribute (not as property), because I think those *niche/power users* will likely use their own instance of `RequestsMock`. Then it will be simple to exchange the registry like any other attribute. Changing the default mock is dangerous as shown with the leaking `CrashRegistry` above. So I would suggest to only allow those things on self created instances.

@beliaev-maksim, I thank you very much for starting this, so I do not want to take your credits and start my own PR on this. Have a look at https://github.com/carloshorn/responses/tree/feature-registry to see my implementation proposal.

While thinking about module level properties, I wondered why `response_callback` is exposed. It will always be `None`, because it is just a reference to the default value. Changing registry is somehow related to changing the response_callback. It is something which should not be done on the default mock, because it could leak into other tests. Global objects can easily become dangerous.
beliaev-maksim: @carloshorn 
you probably missed my last commits. After you made a post about Registry leakage, I re-implemented it. Now registry is only valid within a single test or inside of `RequestMock`

I just pushed updated docs, you can see that there is no leakage any more
beliaev-maksim: @markstory 

sorry for the long thread here. But feature required some brainstorming :)
can you please review code and documentation again?
beliaev-maksim: @markstory 
can you please review this PR?
I have some further changes that might depend on this PR
beliaev-maksim: @markstory 
I resolved conflicts caused by recent updates to master
since 0.16 is scheduled, I bumped the version here to 0.17

as I mentioned, current PR is fully backwards compatible

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index abb7fe7..3446489 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -63,8 +63,8 @@ jobs:
     - name: Run Pytest
       run: |
         # Run test
-        pytest . --cov-report term-missing --cov responses
+        pytest . --cov-report term-missing --cov-report xml --cov responses
 
     - name: Code Coverage Report
       if: success()
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v2
diff --git a/CHANGES b/CHANGES
index c9c9f80..a90606b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,9 @@
 * Deprecate ``match_querystring`` argument in ``Response`` and ``CallbackResponse``.
   Use `responses.matchers.query_param_matcher` or `responses.matchers.query_string_matcher`
 * Added support for non-UTF-8 bytes in `responses.matchers.multipart_matcher`
+* Added `responses.registries`. Now user can create custom registries to
+  manipulate the order of responses in the match algorithm
+  `responses.activate(registry=CustomRegistry)`
 
 0.16.0
 ------
diff --git a/README.rst b/README.rst
index 73b72de..6fccf05 100644
--- a/README.rst
+++ b/README.rst
@@ -387,6 +387,49 @@ include any additional headers.
         resp = session.send(prepped)
         assert resp.text == "hello world"
 
+Response Registry
+---------------------------
+
+By default, ``responses`` will search all registered``Response`` objects and
+return a match. If only one ``Response`` is registered, the registry is kept unchanged.
+However, if multiple matches are found for the same request, then first match is returned and
+removed from registry.
+
+Such behavior is suitable for most of use cases, but to handle special conditions, you can
+implement custom registry which must follow interface of ``registries.FirstMatchRegistry``.
+Redefining the ``find`` method will allow you to create custom search logic and return
+appropriate ``Response``
+
+Example that shows how to set custom registry
+
+.. code-block:: python
+
+    import responses
+    from responses import registries
+
+
+    class CustomRegistry(registries.FirstMatchRegistry):
+        pass
+
+
+    """ Before tests: <responses.registries.FirstMatchRegistry object> """
+
+    # using function decorator
+    @responses.activate(registry=CustomRegistry)
+    def run():
+        """ Within test: <__main__.CustomRegistry object> """
+
+    run()
+    """ After test: <responses.registries.FirstMatchRegistry object> """
+
+    # using context manager
+    with responses.RequestsMock(registry=CustomRegistry) as rsps:
+        """ In context manager: <__main__.CustomRegistry object> """
+
+    """
+    After exit from context manager: <responses.registries.FirstMatchRegistry object>
+    """
+
 Dynamic Responses
 -----------------
 
diff --git a/responses/__init__.py b/responses/__init__.py
index 65cadc7..b8c596b 100644
--- a/responses/__init__.py
+++ b/responses/__init__.py
@@ -16,6 +16,7 @@ from requests.exceptions import ConnectionError
 from requests.utils import cookiejar_from_dict
 from responses.matchers import json_params_matcher as _json_params_matcher
 from responses.matchers import urlencoded_params_matcher as _urlencoded_params_matcher
+from responses.registries import FirstMatchRegistry
 from responses.matchers import query_string_matcher as _query_string_matcher
 from warnings import warn
 
@@ -164,7 +165,7 @@ def wrapper%(wrapper_args)s:
 """
 
 
-def get_wrapped(func, responses):
+def get_wrapped(func, responses, registry=None):
     if six.PY2:
         args, a, kw, defaults = inspect.getargspec(func)
         wrapper_args = inspect.formatargspec(args, a, kw, defaults)
@@ -201,6 +202,9 @@ def get_wrapped(func, responses):
         signature = signature.replace(parameters=params_without_defaults)
         func_args = str(signature)
 
+    if registry is not None:
+        responses._set_registry(registry)
+
     evaldict = {"func": func, "responses": responses}
     six.exec_(
         _wrapper_template % {"wrapper_args": wrapper_args, "func_args": func_args},
@@ -563,9 +567,11 @@ class RequestsMock(object):
         response_callback=None,
         passthru_prefixes=(),
         target="requests.adapters.HTTPAdapter.send",
+        registry=FirstMatchRegistry,
     ):
         self._calls = CallList()
         self.reset()
+        self._registry = registry()  # call only after reset
         self.assert_all_requests_are_fired = assert_all_requests_are_fired
         self.response_callback = response_callback
         self.passthru_prefixes = tuple(passthru_prefixes)
@@ -573,8 +579,21 @@ class RequestsMock(object):
         self._patcher = None
         self._matches = []
 
+    def _get_registry(self):
+        return self._registry
+
+    def _set_registry(self, new_registry):
+        if self.registered():
+            err_msg = (
+                "Cannot replace Registry, current registry has responses.\n"
+                "Run 'responses.registry.reset()' first"
+            )
+            raise AttributeError(err_msg)
+
+        self._registry = new_registry()
+
     def reset(self):
-        self._matches = []
+        self._registry = FirstMatchRegistry()
         self._calls.reset()
         self.passthru_prefixes = ()
 
@@ -616,13 +635,13 @@ class RequestsMock(object):
 
         """
         if isinstance(method, BaseResponse):
-            self._matches.append(method)
+            self._registry.add(method)
             return
 
         if adding_headers is not None:
             kwargs.setdefault("headers", adding_headers)
 
-        self._matches.append(Response(method=method, url=url, body=body, **kwargs))
+        self._registry.add(Response(method=method, url=url, body=body, **kwargs))
 
     def add_passthru(self, prefix):
         """
@@ -657,8 +676,7 @@ class RequestsMock(object):
         else:
             response = BaseResponse(method=method_or_response, url=url)
 
-        while response in self._matches:
-            self._matches.remove(response)
+        self._registry.remove(response)
 
     def replace(self, method_or_response=None, url=None, body="", *args, **kwargs):
         """
@@ -676,11 +694,7 @@ class RequestsMock(object):
         else:
             response = Response(method=method_or_response, url=url, body=body, **kwargs)
 
-        try:
-            index = self._matches.index(response)
-        except ValueError:
-            raise ValueError("Response is not registered for URL %s" % url)
-        self._matches[index] = response
+        self._registry.replace(response)
 
     def upsert(self, method_or_response=None, url=None, body="", *args, **kwargs):
         """
@@ -709,7 +723,7 @@ class RequestsMock(object):
         # ensure the url has a default path set if the url is a string
         # url = _ensure_url_default_path(url, match_querystring)
 
-        self._matches.append(
+        self._registry.add(
             CallbackResponse(
                 url=url,
                 method=method,
@@ -721,7 +735,7 @@ class RequestsMock(object):
         )
 
     def registered(self):
-        return self._matches
+        return self._registry.registered
 
     @property
     def calls(self):
@@ -737,8 +751,14 @@ class RequestsMock(object):
         self.reset()
         return success
 
-    def activate(self, func):
-        return get_wrapped(func, self)
+    def activate(self, func=None, registry=None):
+        if func is not None:
+            return get_wrapped(func, self)
+
+        def deco_activate(func):
+            return get_wrapped(func, self, registry)
+
+        return deco_activate
 
     def _find_match(self, request):
         """
@@ -749,21 +769,7 @@ class RequestsMock(object):
             (Response) found match. If multiple found, then remove & return the first match.
             (list) list with reasons why other matches don't match
         """
-        found = None
-        found_match = None
-        match_failed_reasons = []
-        for i, match in enumerate(self._matches):
-            match_result, reason = match.matches(request)
-            if match_result:
-                if found is None:
-                    found = i
-                    found_match = match
-                else:
-                    # Multiple matches found.  Remove & return the first match.
-                    return self._matches.pop(found), match_failed_reasons
-            else:
-                match_failed_reasons.append(reason)
-        return found_match, match_failed_reasons
+        return self._registry.find(request)
 
     def _parse_request_params(self, url):
         params = {}
@@ -802,7 +808,7 @@ class RequestsMock(object):
                 "- %s %s\n\n"
                 "Available matches:\n" % (request.method, request.url)
             )
-            for i, m in enumerate(self._matches):
+            for i, m in enumerate(self.registered()):
                 error_msg += "- {} {} {}\n".format(
                     m.method, m.url, match_failed_reasons[i]
                 )
@@ -846,7 +852,7 @@ class RequestsMock(object):
         if not allow_assert:
             return
 
-        not_called = [m for m in self._matches if m.call_count == 0]
+        not_called = [m for m in self.registered() if m.call_count == 0]
         if not_called:
             raise AssertionError(
                 "Not all requests have been executed {0!r}".format(
diff --git a/responses/__init__.pyi b/responses/__init__.pyi
index 0a20ad8..a0af8f2 100644
--- a/responses/__init__.pyi
+++ b/responses/__init__.pyi
@@ -23,6 +23,7 @@ from unittest import mock as std_mock
 from urllib.parse import quote as quote
 from urllib3.response import HTTPHeaderDict
 from .matchers import urlencoded_params_matcher, json_params_matcher
+from .registries import FirstMatchRegistry
 
 
 def _clean_unicode(url: str) -> str: ...
@@ -38,7 +39,7 @@ def _handle_body(
 def _has_unicode(s: str) -> bool: ...
 def _is_string(s: Union[Pattern[str], str]) -> bool: ...
 def get_wrapped(
-    func: Callable[..., Any], responses: RequestsMock
+    func: Callable[..., Any], responses: RequestsMock, registry: Optional[Any]
 ) -> Callable[..., Any]: ...
 
 
@@ -146,6 +147,8 @@ class OriginalResponseShim:
     ) -> None: ...
     def isclosed(self) -> bool: ...
 
+_F = TypeVar("_F", bound=Callable[..., Any])
+
 class RequestsMock:
     DELETE: Literal["DELETE"]
     GET: Literal["GET"]
@@ -165,6 +168,7 @@ class RequestsMock:
         response_callback: Optional[Callable[[Any], Any]] = ...,
         passthru_prefixes: Tuple[str, ...] = ...,
         target: str = ...,
+        registry: Any = ...,
     ) -> None:
         self._patcher = Callable[[Any], Any]
         self._calls = CallList
@@ -184,19 +188,17 @@ class RequestsMock:
     def calls(self) -> CallList: ...
     def __enter__(self) -> RequestsMock: ...
     def __exit__(self, type: Any, value: Any, traceback: Any) -> bool: ...
-    activate: _Activate
+    def activate(self, func: Optional[_F], registry: Optional[Any]) -> _F: ...
     def start(self) -> None: ...
     def stop(self, allow_assert: bool = ...) -> None: ...
     def assert_call_count(self, url: str, count: int) -> bool: ...
     def registered(self) -> List[Any]: ...
+    def _set_registry(self, registry: Any) -> None: ...
+    def _get_registry(self) -> Any: ...
 
-_F = TypeVar("_F", bound=Callable[..., Any])
 
 HeaderSet = Optional[Union[Mapping[str, str], List[Tuple[str, str]]]]
 
-class _Activate(Protocol):
-    def __call__(self, func: _F) -> _F: ...
-
 class _Add(Protocol):
     def __call__(
         self,
@@ -273,7 +275,7 @@ class _Registered(Protocol):
     def __call__(self) -> List[Response]: ...
 
 
-activate: _Activate
+activate: Any
 add: _Add
 add_callback: _AddCallback
 add_passthru: _AddPassthru
@@ -328,5 +330,5 @@ __all__ = [
     "start",
     "stop",
     "target",
-    "upsert"
+    "upsert",
 ]
diff --git a/responses/registries.py b/responses/registries.py
new file mode 100644
index 0000000..6546798
--- /dev/null
+++ b/responses/registries.py
@@ -0,0 +1,43 @@
+class FirstMatchRegistry(object):
+    def __init__(self):
+        self._responses = []
+
+    @property
+    def registered(self):
+        return self._responses
+
+    def reset(self):
+        self._responses = []
+
+    def find(self, request):
+        found = None
+        found_match = None
+        match_failed_reasons = []
+        for i, response in enumerate(self.registered):
+            match_result, reason = response.matches(request)
+            if match_result:
+                if found is None:
+                    found = i
+                    found_match = response
+                else:
+                    # Multiple matches found.  Remove & return the first response.
+                    return self.registered.pop(found), match_failed_reasons
+            else:
+                match_failed_reasons.append(reason)
+        return found_match, match_failed_reasons
+
+    def add(self, response):
+        self.registered.append(response)
+
+    def remove(self, response):
+        while response in self.registered:
+            self.registered.remove(response)
+
+    def replace(self, response):
+        try:
+            index = self.registered.index(response)
+        except ValueError:
+            raise ValueError(
+                "Response is not registered for URL {}".format(response.url)
+            )
+        self.registered[index] = response
diff --git a/responses/registries.pyi b/responses/registries.pyi
new file mode 100644
index 0000000..7be9294
--- /dev/null
+++ b/responses/registries.pyi
@@ -0,0 +1,17 @@
+from typing import (
+    List,
+    Tuple,
+)
+from requests.adapters import PreparedRequest
+from responses import BaseResponse
+
+class FirstMatchRegistry:
+    _responses = List[BaseResponse]
+    def __init__(self) -> None: ...
+    @property
+    def registered(self) -> List[BaseResponse]: ...
+    def reset(self) -> None: ...
+    def find(self, request: PreparedRequest) -> Tuple[BaseResponse, List[str]]: ...
+    def add(self, response: BaseResponse) -> None: ...
+    def remove(self, response: BaseResponse) -> None: ...
+    def replace(self, response: BaseResponse) -> None: ...


Test patch:
diff --git a/responses/test_matchers.py b/responses/test_matchers.py
index 4587b28..92a9003 100644
--- a/responses/test_matchers.py
+++ b/responses/test_matchers.py
@@ -18,7 +18,7 @@ def assert_response(resp, body=None, content_type="text/plain"):
 
 
 def assert_reset():
-    assert len(responses._default_mock._matches) == 0
+    assert len(responses._default_mock.registered()) == 0
     assert len(responses.calls) == 0
 
 
diff --git a/responses/test_responses.py b/responses/test_responses.py
index 164143e..e076ec7 100644
--- a/responses/test_responses.py
+++ b/responses/test_responses.py
@@ -18,6 +18,7 @@ from responses import (
     PassthroughResponse,
     matchers,
     CallbackResponse,
+    registries,
 )
 
 
@@ -28,7 +29,7 @@ except ImportError:
 
 
 def assert_reset():
-    assert len(responses._default_mock._matches) == 0
+    assert len(responses._default_mock.registered()) == 0
     assert len(responses.calls) == 0
 
 
@@ -1070,24 +1071,24 @@ def test_assert_all_requests_are_fired():
         # check that assert_all_requests_are_fired=True doesn't remove urls
         with responses.RequestsMock(assert_all_requests_are_fired=True) as m:
             m.add(responses.GET, "http://example.com", body=b"test")
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
             requests.get("http://example.com")
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
 
         # check that assert_all_requests_are_fired=True counts mocked errors
         with responses.RequestsMock(assert_all_requests_are_fired=True) as m:
             m.add(responses.GET, "http://example.com", body=Exception())
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
             with pytest.raises(Exception):
                 requests.get("http://example.com")
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
 
         with responses.RequestsMock(assert_all_requests_are_fired=True) as m:
             m.add_callback(responses.GET, "http://example.com", request_callback)
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
             with pytest.raises(BaseException):
                 requests.get("http://example.com")
-            assert len(m._matches) == 1
+            assert len(m.registered()) == 1
 
     run()
     assert_reset()
@@ -1825,7 +1826,7 @@ def test_mocked_responses_list_registered():
 
         mocks_list = responses.registered()
 
-        assert mocks_list == responses.mock._matches
+        assert mocks_list == responses.mock.registered()
         assert mocks_list == [first_response, second_response, third_response]
 
     run()
@@ -1850,3 +1851,68 @@ def test_rfc_compliance(url, other_url):
 
     run()
     assert_reset()
+
+
+def test_set_registry_not_empty():
+    class CustomRegistry(registries.FirstMatchRegistry):
+        pass
+
+    @responses.activate
+    def run():
+        url = "http://fizzbuzz/foo"
+        responses.add(method=responses.GET, url=url)
+        with pytest.raises(AttributeError) as excinfo:
+            responses.mock._set_registry(CustomRegistry)
+        msg = str(excinfo.value)
+        assert "Cannot replace Registry, current registry has responses" in msg
+
+    run()
+    assert_reset()
+
+
+def test_set_registry():
+    class CustomRegistry(registries.FirstMatchRegistry):
+        pass
+
+    @responses.activate(registry=CustomRegistry)
+    def run_with_registry():
+        assert type(responses.mock._get_registry()) == CustomRegistry
+
+    @responses.activate
+    def run():
+        # test that registry does not leak to another test
+        assert type(responses.mock._get_registry()) == registries.FirstMatchRegistry
+
+    run_with_registry()
+    run()
+    assert_reset()
+
+
+def test_set_registry_context_manager():
+    def run():
+        class CustomRegistry(registries.FirstMatchRegistry):
+            pass
+
+        with responses.RequestsMock(
+            assert_all_requests_are_fired=False, registry=CustomRegistry
+        ) as rsps:
+            assert type(rsps._get_registry()) == CustomRegistry
+            assert type(responses.mock._get_registry()) == registries.FirstMatchRegistry
+
+    run()
+    assert_reset()
+
+
+def test_registry_reset():
+    def run():
+        class CustomRegistry(registries.FirstMatchRegistry):
+            pass
+
+        with responses.RequestsMock(
+            assert_all_requests_are_fired=False, registry=CustomRegistry
+        ) as rsps:
+            rsps._get_registry().reset()
+            assert not rsps.registered()
+
+    run()
+    assert_reset()