diff --git a/airbyte_cdk/sources/streams/call_rate.py b/airbyte_cdk/sources/streams/call_rate.py index 4a06db3b22..76699b00ae 100644 --- a/airbyte_cdk/sources/streams/call_rate.py +++ b/airbyte_cdk/sources/streams/call_rate.py @@ -120,7 +120,7 @@ def __init__( if url: parsed_url = parse.urlsplit(url) url_base = f"{parsed_url.scheme}://{parsed_url.netloc}" - url_path = parsed_url.path if parsed_url.path != "/" else None + url_path = parsed_url.path.rstrip("/") or None else: url_base = None url_path = None @@ -129,7 +129,7 @@ def __init__( self._regex_matcher = HttpRequestRegexMatcher( method=method, url_base=url_base, - url_path_pattern=re.escape(url_path) if url_path else None, + url_path_pattern=re.escape(url_path) + r"(?=/|$)" if url_path else None, params=params, headers=headers, ) @@ -171,7 +171,7 @@ def __init__( """ :param method: HTTP method (e.g. "GET", "POST"); compared case-insensitively. :param url_base: Base URL (scheme://host) that must match. - :param url_path_pattern: A regex pattern that will be applied to the path portion of the URL. + :param url_path_pattern: A regex pattern matched against the path with any trailing slash removed, and also against the original path so patterns requiring a trailing slash still work. :param params: Dictionary of query parameters that must be present in the request. :param headers: Dictionary of headers that must be present (header keys are compared case-insensitively). :param weight: The weight of a request matching this matcher. If set, this value is used @@ -224,7 +224,8 @@ def __call__(self, request: Any) -> bool: # Reconstruct the base: scheme://netloc request_url_base = f"{str(parsed_url.scheme)}://{str(parsed_url.netloc)}" # The path (without query parameters) - request_path = str(parsed_url.path).rstrip("/") + request_path = str(parsed_url.path) + normalized_path = request_path.rstrip("/") # If a base URL is provided, check that it matches. if self._url_base is not None: @@ -233,7 +234,10 @@ def __call__(self, request: Any) -> bool: # If a URL path pattern is provided, ensure the path matches the regex. if self._url_path_pattern is not None: - if not self._url_path_pattern.search(request_path): + if not ( + self._url_path_pattern.search(normalized_path) + or (request_path != normalized_path and self._url_path_pattern.search(request_path)) + ): return False # Check query parameters. diff --git a/unit_tests/sources/streams/test_call_rate.py b/unit_tests/sources/streams/test_call_rate.py index a423fe5737..0c96a03f50 100644 --- a/unit_tests/sources/streams/test_call_rate.py +++ b/unit_tests/sources/streams/test_call_rate.py @@ -72,6 +72,25 @@ def test_url(self, request_factory): assert not matcher(request_factory(url="http://some_wrong_url")) assert matcher(request_factory(url="http://some_url")) + @pytest.mark.parametrize("matcher_url", ["http://some_url/users", "http://some_url/users/"]) + @pytest.mark.parametrize("request_url", ["http://some_url/users", "http://some_url/users/"]) + @try_all_types_of_requests + def test_url_trailing_slash(self, request_factory, matcher_url, request_url): + matcher = HttpRequestMatcher(url=matcher_url) + assert matcher(request_factory(url=request_url)) + assert not matcher(request_factory(url="http://some_url/other")) + + root_matcher = HttpRequestMatcher(url="http://some_url/") + assert root_matcher(request_factory(url="http://some_url")) + assert root_matcher(request_factory(url="http://some_url/anything")) + + @try_all_types_of_requests + def test_url_path_boundary(self, request_factory): + matcher = HttpRequestMatcher(url="http://some_url/users/") + assert not matcher(request_factory(url="http://some_url/users-other")) + assert not matcher(request_factory(url="http://some_url/users_other")) + assert matcher(request_factory(url="http://some_url/users/123")) + @try_all_types_of_requests def test_method(self, request_factory): matcher = HttpRequestMatcher(method="GET") @@ -514,6 +533,47 @@ def test_url_path_pattern(self): assert matcher(req_ok) assert not matcher(req_wrong) + @pytest.mark.parametrize( + "pattern,matching_urls,non_matching_urls", + [ + (r"^/users$", ["/users", "/users/"], ["/users/123"]), + (r"^/users/$", ["/users/"], ["/users", "/users/123"]), + (r"^/users/?$", ["/users", "/users/"], ["/users/123"]), + ], + ) + def test_url_path_pattern_trailing_slash(self, pattern, matching_urls, non_matching_urls): + matcher = HttpRequestRegexMatcher(url_path_pattern=pattern) + + for path in matching_urls: + assert matcher(Request("GET", f"https://example.com{path}")) + for path in non_matching_urls: + assert not matcher(Request("GET", f"https://example.com{path}")) + + def test_api_budget_matches_trailing_slash_request(self): + regex_policy = MovingWindowCallRatePolicy( + rates=[Rate(limit=1, interval=timedelta(minutes=1))], + matchers=[HttpRequestRegexMatcher(url_path_pattern=r"^/users/$")], + ) + regex_budget = APIBudget(policies=[regex_policy]) + regex_request = Request("GET", "https://example.com/users/") + + assert regex_budget.get_matching_policy(regex_request) is regex_policy + regex_budget.acquire_call(regex_request, block=False) + with pytest.raises(CallRateLimitHit): + regex_budget.acquire_call(regex_request, block=False) + + literal_policy = MovingWindowCallRatePolicy( + rates=[Rate(limit=1, interval=timedelta(minutes=1))], + matchers=[HttpRequestMatcher(url="https://example.com/users/")], + ) + literal_budget = APIBudget(policies=[literal_policy]) + literal_request = Request("GET", "https://example.com/users") + + assert literal_budget.get_matching_policy(literal_request) is literal_policy + literal_budget.acquire_call(literal_request, block=False) + with pytest.raises(CallRateLimitHit): + literal_budget.acquire_call(literal_request, block=False) + def test_query_params(self): matcher = HttpRequestRegexMatcher(params={"foo": "bar"})