diff --git a/src/crawlee/_types.py b/src/crawlee/_types.py index 7a1e0d7eef..77bbc5ec98 100644 --- a/src/crawlee/_types.py +++ b/src/crawlee/_types.py @@ -168,10 +168,16 @@ class EnqueueLinksKwargs(TypedDict): """ include: NotRequired[Sequence[re.Pattern | Glob]] - """List of regular expressions or globs that URLs must match to be enqueued.""" + """List of regular expressions or globs that URLs must match to be enqueued. + + Regexes match anywhere in the URL and globs are case-insensitive, aligned with crawlee-js. + """ exclude: NotRequired[Sequence[re.Pattern | Glob]] - """List of regular expressions or globs that URLs must not match to be enqueued.""" + """List of regular expressions or globs that URLs must not match to be enqueued. + + Regexes match anywhere in the URL and globs are case-insensitive, aligned with crawlee-js. + """ class AddRequestsKwargs(EnqueueLinksKwargs): diff --git a/src/crawlee/_utils/globs.py b/src/crawlee/_utils/globs.py index ab352113a3..3d46b9dc86 100644 --- a/src/crawlee/_utils/globs.py +++ b/src/crawlee/_utils/globs.py @@ -9,11 +9,16 @@ class Glob: - """Wraps a glob pattern (supports the `*`, `**`, `?` wildcards).""" + """Wraps a glob pattern (supports the `*`, `**`, `?` wildcards). + + Matching is case-insensitive, aligned with crawlee-js (`Minimatch` with `nocase: true`). + """ def __init__(self, glob: str) -> None: self.glob = glob - self.regexp = re.compile(_translate(self.glob, recursive=True)) + # `re.IGNORECASE` mirrors Minimatch's `nocase: true`. The `\A` anchor keeps the pattern a + # full-string match, so `search()` (like JS `regexp.test`) cannot match a substring only. + self.regexp = re.compile(rf'\A{_translate(self.glob, recursive=True)}', re.IGNORECASE) def _translate( diff --git a/src/crawlee/crawlers/_basic/_basic_crawler.py b/src/crawlee/crawlers/_basic/_basic_crawler.py index 96ff205350..94e1b6395d 100644 --- a/src/crawlee/crawlers/_basic/_basic_crawler.py +++ b/src/crawlee/crawlers/_basic/_basic_crawler.py @@ -1075,8 +1075,12 @@ async def enqueue_links( def _enqueue_links_filter_iterator( self, request_iterator: Iterator[TRequestIterator], origin_url: str, **kwargs: Unpack[EnqueueLinksKwargs] ) -> Iterator[TRequestIterator]: - """Filter requests based on the enqueue strategy and URL patterns.""" - limit = kwargs.get('limit') + """Filter requests based on the enqueue strategy and URL patterns. + + The `limit` kwarg is intentionally not applied here - it counts enqueued requests, so it is + enforced after `transform_request_function` (and any other pre-enqueue skipping) runs, aligned + with crawlee-js, where the limit is applied to the final request list. + """ parsed_origin_url = URL(origin_url) strategy = kwargs.get('strategy', 'all') @@ -1113,24 +1117,22 @@ def _enqueue_links_filter_iterator( if self._check_url_patterns(target_url, kwargs.get('include'), kwargs.get('exclude')): yield request - if limit is not None: - limit -= 1 - if limit <= 0: - break - def _check_url_patterns( self, target_url: str, include: Sequence[re.Pattern[Any] | Glob] | None, exclude: Sequence[re.Pattern[Any] | Glob] | None, ) -> bool: - """Check if a URL matches configured include/exclude patterns.""" + """Check if a URL matches configured include/exclude patterns. + + Patterns are matched with `search` (unanchored), aligned with crawlee-js (`regexp.test`). + """ # If the URL matches any `exclude` pattern, reject it for pattern in exclude or (): if isinstance(pattern, Glob): pattern = pattern.regexp # noqa: PLW2901 - if pattern.match(target_url) is not None: + if pattern.search(target_url) is not None: return False # If there are no `include` patterns and the URL passed all `exclude` patterns, accept the URL @@ -1142,7 +1144,7 @@ def _check_url_patterns( if isinstance(pattern, Glob): pattern = pattern.regexp # noqa: PLW2901 - if pattern.match(target_url) is not None: + if pattern.search(target_url) is not None: return True # The URL does not match any `include` pattern - reject it @@ -1378,6 +1380,20 @@ async def _add_requests( if self._max_crawl_depth is None or dst_request.crawl_depth <= self._max_crawl_depth: context_aware_requests.append(dst_request) + # The `limit` counts requests actually enqueued (i.e. newly added to the request manager), so it is + # applied as the last step, after all filters and `transform_request_function` skipping, aligned with + # crawlee-js, where the budget is decremented only for requests that were not in the queue already. + limit = kwargs.get('limit') + if limit is not None: + remaining_budget = limit + for dst_request in context_aware_requests: + if remaining_budget <= 0: + break + processed_request = await request_manager.add_request(dst_request) + if processed_request is not None and not processed_request.was_already_present: + remaining_budget -= 1 + return None + return await request_manager.add_requests(context_aware_requests) async def _commit_request_handler_result(self, context: BasicCrawlingContext) -> None: diff --git a/src/crawlee/request_loaders/_sitemap_request_loader.py b/src/crawlee/request_loaders/_sitemap_request_loader.py index a528753e6d..3c4455684f 100644 --- a/src/crawlee/request_loaders/_sitemap_request_loader.py +++ b/src/crawlee/request_loaders/_sitemap_request_loader.py @@ -326,13 +326,16 @@ def _check_url_patterns( include: Sequence[re.Pattern[Any] | Glob] | None, exclude: Sequence[re.Pattern[Any] | Glob] | None, ) -> bool: - """Check if a URL matches configured include/exclude patterns.""" + """Check if a URL matches configured include/exclude patterns. + + Patterns are matched with `search` (unanchored), aligned with crawlee-js (`url.match`). + """ # If the URL matches any `exclude` pattern, reject it for pattern in exclude or (): if isinstance(pattern, Glob): pattern = pattern.regexp # noqa: PLW2901 - if pattern.match(target_url) is not None: + if pattern.search(target_url) is not None: return False # If there are no `include` patterns and the URL passed all `exclude` patterns, accept the URL @@ -344,7 +347,7 @@ def _check_url_patterns( if isinstance(pattern, Glob): pattern = pattern.regexp # noqa: PLW2901 - if pattern.match(target_url) is not None: + if pattern.search(target_url) is not None: return True # The URL does not match any `include` pattern - reject it diff --git a/tests/unit/_utils/test_globs.py b/tests/unit/_utils/test_globs.py index 970678cfed..b81a88fa02 100644 --- a/tests/unit/_utils/test_globs.py +++ b/tests/unit/_utils/test_globs.py @@ -15,3 +15,16 @@ def test_double_asteritsk() -> None: assert glob.regexp.match('bar/') is None assert glob.regexp.match('foo/bar') is not None assert glob.regexp.match('foo/bar/baz') is not None + + +def test_case_insensitive() -> None: + glob = Glob('https://Someplace.com/**/cats') + assert glob.regexp.search('https://someplace.com/blog/category/cats') is not None + assert glob.regexp.search('https://Someplace.com/blog/category/cats') is not None + + +def test_search_matches_whole_string_only() -> None: + # URL filters match with `search` (aligned with `regexp.test` in crawlee-js), so the pattern must + # stay anchored to the whole string - it must not match a URL merely containing the pattern. + glob = Glob('https://example.com/*') + assert glob.regexp.search('https://evil.com/redirect?to=https://example.com/x') is None diff --git a/tests/unit/crawlers/_basic/test_basic_crawler.py b/tests/unit/crawlers/_basic/test_basic_crawler.py index 56ba257e86..fc8de395cd 100644 --- a/tests/unit/crawlers/_basic/test_basic_crawler.py +++ b/tests/unit/crawlers/_basic/test_basic_crawler.py @@ -703,6 +703,36 @@ class AddRequestsTestInput: ), id='include_exclude_3', ), + pytest.param( + AddRequestsTestInput( + start_url=INCLUDE_TEST_URLS[0], + loaded_url=INCLUDE_TEST_URLS[0], + requests=INCLUDE_TEST_URLS, + kwargs=EnqueueLinksKwargs(include=[Glob('https://SOMEPLACE.com/**/cats')]), + expected_urls=[INCLUDE_TEST_URLS[1], INCLUDE_TEST_URLS[4]], + ), + id='include_glob_case_insensitive', + ), + pytest.param( + AddRequestsTestInput( + start_url=INCLUDE_TEST_URLS[0], + loaded_url=INCLUDE_TEST_URLS[0], + requests=INCLUDE_TEST_URLS, + kwargs=EnqueueLinksKwargs(include=[re.compile(r'/category/cats')]), + expected_urls=[INCLUDE_TEST_URLS[1]], + ), + id='include_regex_unanchored', + ), + pytest.param( + AddRequestsTestInput( + start_url=INCLUDE_TEST_URLS[0], + loaded_url=INCLUDE_TEST_URLS[0], + requests=INCLUDE_TEST_URLS, + kwargs=EnqueueLinksKwargs(exclude=[re.compile(r'/archive/')]), + expected_urls=[INCLUDE_TEST_URLS[1], INCLUDE_TEST_URLS[2]], + ), + id='exclude_regex_unanchored', + ), ], ) async def test_enqueue_strategy(test_input: AddRequestsTestInput) -> None: @@ -729,6 +759,46 @@ async def handler(context: BasicCrawlingContext) -> None: assert visited == set(test_input.expected_urls) +async def test_add_requests_limit_skips_duplicates_before_counting() -> None: + """`limit` counts only requests newly added to the request manager, not duplicate candidates. + + Requests already present in the queue must not consume the limit, so unique requests later in + the list are still enqueued (aligned with crawlee-js, where the budget is decremented only for + requests that were not already in the queue). + """ + visited = Mock() + + crawler = BasicCrawler() + + @crawler.router.handler('start') + async def start_handler(context: BasicCrawlingContext) -> None: + await context.add_requests(['https://someplace.com/first']) + # The list contains the URL enqueued above (already in the queue), a duplicate of it, and + # two new ones. With `limit=2` the two new URLs must both be enqueued. + await context.add_requests( + [ + 'https://someplace.com/first', + 'https://someplace.com/first', + 'https://someplace.com/second', + 'https://someplace.com/third', + ], + limit=2, + ) + + @crawler.router.default_handler + async def handler(context: BasicCrawlingContext) -> None: + visited(context.request.url) + + await crawler.run([Request.from_url('https://someplace.com/', label='start')]) + + visited_urls = {call[0][0] for call in visited.call_args_list} + assert visited_urls == { + 'https://someplace.com/first', + 'https://someplace.com/second', + 'https://someplace.com/third', + } + + async def test_session_rotation(server_url: URL) -> None: session_ids: list[str | None] = [] diff --git a/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py b/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py index 261c45c1e4..782f49738f 100644 --- a/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py +++ b/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py @@ -171,6 +171,31 @@ async def request_handler(context: BeautifulSoupCrawlingContext) -> None: assert headers[3]['transform-header'] == 'my-header' +async def test_enqueue_links_limit_counts_enqueued_requests(server_url: URL, http_client: HttpClient) -> None: + crawler = BeautifulSoupCrawler(http_client=http_client) + visited: list[str] = [] + + def transform_skip_two( + request_options: RequestOptions, + ) -> RequestOptions | RequestTransformAction: + if 'page_2' in request_options['url'] or 'page_3' in request_options['url']: + return 'skip' + return request_options + + @crawler.router.default_handler + async def request_handler(context: BeautifulSoupCrawlingContext) -> None: + visited.append(context.request.url) + if 'sub_index' in context.request.url: + # `/sub_index` links to `/page_3`, `/page_2` and `/base_page`. The transform skips + # the first two, so with `limit=2` the remaining `/base_page` must still be enqueued - + # the limit counts enqueued requests, not extracted ones (aligned with crawlee-js). + await context.enqueue_links(transform_request_function=transform_skip_two, limit=2) + + await crawler.run([str(server_url / 'sub_index')]) + + assert str(server_url / 'base_page') in visited + + async def test_handle_blocked_request(server_url: URL, http_client: HttpClient) -> None: crawler = BeautifulSoupCrawler(max_session_rotations=1, http_client=http_client) stats = await crawler.run([str(server_url / 'incapsula')]) diff --git a/tests/unit/request_loaders/test_sitemap_request_loader.py b/tests/unit/request_loaders/test_sitemap_request_loader.py index e39e2a04fb..326c902aa7 100644 --- a/tests/unit/request_loaders/test_sitemap_request_loader.py +++ b/tests/unit/request_loaders/test_sitemap_request_loader.py @@ -1,5 +1,6 @@ import base64 import gzip +import re from contextlib import asynccontextmanager from typing import TYPE_CHECKING, Any, cast from unittest.mock import AsyncMock, MagicMock, patch @@ -471,6 +472,30 @@ async def test_sitemap_loader_filters_cross_host_urls(server_url: URL, http_clie assert fetched == [same_host_url] +async def test_sitemap_loader_regex_patterns_are_unanchored(server_url: URL, http_client: HttpClient) -> None: + """Regex `include`/`exclude` patterns match anywhere in the URL, like `url.match` in crawlee-js.""" + kept_url = str(server_url / 'catalog/page') + filtered_url = str(server_url / 'archive/page') + sitemap_content = _make_urlset([kept_url, filtered_url]) + sitemap_url = (server_url / 'sitemap.xml').with_query(base64=encode_base64(sitemap_content.encode())) + + loader = SitemapRequestLoader( + [str(sitemap_url)], + http_client=http_client, + include=[re.compile(r'/catalog/')], + exclude=[re.compile(r'/archive/')], + ) + + fetched: list[str] = [] + while not await loader.is_finished(): + request = await loader.fetch_next_request() + if request is not None: + fetched.append(request.url) + await loader.mark_request_as_handled(request) + + assert fetched == [kept_url] + + async def test_sitemap_loader_filters_cross_host_nested_sitemap(server_url: URL, http_client: HttpClient) -> None: """Nested `` entries on a different host are dropped before fetching them.""" child_content = _make_urlset([str(server_url / 'inner')])