Xqt has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1183074?usp=email )

Change subject: site.allpages: apply client-side filtering for maxsize in misermode ......................................................................

site.allpages: apply client-side filtering for maxsize in misermode

MediaWiki ignores `apmaxsize` when $wgMiserMode is enabled, which caused `site.allpages(maxsize=...)` to yield unfiltered results

- Add `APIGeneratorBase.filter_func` and `APIGeneratorBase.filter_item` filter function in APIGeneratorBase - Call this filter in all generators of APIGeneratorBase subclasses. - Apply client-side filtering via `APIGeneratorBase.filter_func` if *maxsize* is set and the site runs in misermode. - Ensures page content is always loaded in this case, regardless of the *content* parameter, so that page lengths are available for filtering. - Marks all parameters except *start* as keyword-only for clarity. - remove `mysite.data_repository() == mysite` in test_allpages_pagesize test method which should be obsolete.

Bug: T402995 Change-Id: If0cc80fb2047396a51feb6b986d4bec4c68d4643 --- M pywikibot/data/api/_generators.py M pywikibot/site/_generators.py M tests/site_generators_tests.py 3 files changed, 159 insertions(+), 38 deletions(-)

Approvals: jenkins-bot: Verified Xqt: Looks good to me, approved

diff --git a/pywikibot/data/api/_generators.py b/pywikibot/data/api/_generators.py index 477dcb7..26795cf 100644 --- a/pywikibot/data/api/_generators.py +++ b/pywikibot/data/api/_generators.py @@ -43,12 +43,67 @@

class APIGeneratorBase(ABC):

- """A wrapper class to handle the usage of the ``parameters`` parameter. + """Base class for all API and query request generators. + + Handles request cleaning and filtering. Each instance can have an + optional filter function applied to items before yielding. Set this + via the :attr:`filter_func` property, which should be a callable + accepting a single item and returning True to yield it, alse to skip + it. If :attr:`filter_func` is None, no filtering is applied. + + Subclasses can override :meth:`filter_item` for more complex + filtering logic.

.. versionchanged:: 7.6 - renamed from _RequestWrapper + Renamed from _RequestWrapper. + .. versionchanged:: 10.4 + Introduced :attr:`filter_func` and :meth:`filter_item` for + instance-level item filtering. """

+ _filter_func: Callable[[Any], bool] | None = None + + @property + def filter_func(self) -> Callable[[Any], bool] | None: + """Get the filter function for this generator instance. + + Returns the instance-specific filter if set, otherwise the + class-level default (None by default). + + .. versionadded:: 10.4 + + :return: Callable that accepts an item and returns True to + yield, False to skip; or None to disable filtering + """ + return getattr(self, '_filter_func', type(self)._filter_func) + + @filter_func.setter + def filter_func(self, func: Callable[[Any], bool] | None): + """Set a filter function to apply to items before yielding. + + .. versionadded:: 10.4 + + :param func: Callable that accepts an item and returns True to + yield, False to skip; or None to disable filtering + """ + self._filter_func = func + + def filter_item(self, item: Any) -> bool: + """Determine if a given item should be yielded. + + By default, applies :attr:`filter_func` if set. Returns True if + no filter is set. + + .. versionadded:: 10.4 + + :param item: The item to check + :return: True if the item should be yielded, False otherwise + """ + if self.filter_func is not None: + return self.filter_func(item) + + return True + def _clean_kwargs(self, kwargs, **mw_api_args): """Clean kwargs, define site and request class.""" if 'site' not in kwargs: @@ -162,13 +217,20 @@ """Submit request and iterate the response.

Continues response as needed until limit (if defined) is reached. + Applies :meth:`filter_item()<APIGeneratorBase.filter_item>` to + each item before yielding.

.. versionchanged:: 7.6 - changed from iterator method to generator property + Changed from iterator method to generator property + .. versionchanged:: 10.4 + Applies `filter_item` for instance-level filtering. + + :yield: Items from the MediaWiki API, filtered by `filter_item()` """ offset = self.starting_offset n = 0 while True: + # Set the continue parameter for the request self.request[self.continue_name] = offset pywikibot.debug(f'{type(self).__name__}: Request: {self.request}') data = self.request.submit() @@ -178,14 +240,17 @@ f'{type(self).__name__}: Retrieved {n_items} items') if n_items > 0: for item in data[self.data_name]: - yield item - n += 1 - if self.limit is not None and n >= self.limit: - pywikibot.debug( - f'{type(self).__name__}: Stopped iterating due to' - ' exceeding item limit.' - ) - return + # Apply the instance filter function before yielding + if self.filter_item(item): + yield item + n += 1 + # Stop iterating if the limit is reached + if self.limit is not None and n >= self.limit: + pywikibot.debug( + f'{type(self).__name__}: Stopped iterating due' + ' to exceeding item limit.' + ) + return offset += n_items else: pywikibot.debug(f'{type(self).__name__}: Stopped iterating' @@ -570,17 +635,36 @@ return resultdata

def _extract_results(self, resultdata): - """Extract results from resultdata.""" + """Extract results from resultdata, applying `filter_item()`. + + :attr:`generator` helper method which yields each result that + passes :meth:`filter_item() <APIGeneratorBase.filter_item>` and + respects namespaces and the generator's limit. + + .. versionchanged:: 10.4 + Applies `filter_item()` for instance-level filtering. + + :param resultdata: List or iterable of raw API items + :yield: Processed items that pass the filter + :raises RuntimeError: if self.limit is reached + + :meta public: + """ for item in resultdata: result = self.result(item) if self._namespaces and not self._check_result_namespace(result): continue

+ # Apply the instance filter before yielding + if not self.filter_item(result): + continue + yield result

modules_item_intersection = set(self.modules) & set(item) if isinstance(item, dict) and modules_item_intersection: - # if we need to count elements contained in items in + # Count elements contained in sub-items. + # If we need to count elements contained in items in # self.data["query"]["pages"], we want to count # item[self.modules] (e.g. 'revisions') and not # self.resultkey (i.e. 'pages') @@ -589,7 +673,8 @@ # otherwise we proceed as usual else: self._count += 1 - # note: self.limit could be -1 + + # Stop if limit is reached; note: self.limit could be -1 if self.limit and 0 < self.limit <= self._count: raise RuntimeError( 'QueryGenerator._extract_results reached the limit') @@ -599,9 +684,15 @@ """Submit request and iterate the response based on self.resultkey.

Continues response as needed until limit (if any) is reached. + Each item is already filtered by `_extract_results()`.

.. versionchanged:: 7.6 - changed from iterator method to generator property + Changed from iterator method to generator property + .. versionchanged:: 10.4 + Items are filtered via :meth:`filter_item() + <APIGeneratorBase.filter_item>` inside :meth:`_extract_results`. + + :yield: Items from the API, already filtered """ previous_result_had_data = True prev_limit = new_limit = None @@ -616,7 +707,7 @@

if not self.data or not isinstance(self.data, dict): pywikibot.debug(f'{type(self).__name__}: stopped iteration' - ' because no dict retrieved from api.') + ' because no dict retrieved from API.') break

if 'query' in self.data and self.resultkey in self.data['query']: @@ -638,13 +729,13 @@ else: if 'query' not in self.data: pywikibot.log(f"{type(self).__name__}: 'query' not found" - ' in api response.') + ' in API response.') pywikibot.log(str(self.data))

# if (query-)continue is present, self.resultkey might not have # been fetched yet if self.continue_name not in self.data: - break # No results. + break # No results

# self.resultkey not in data in last request.submit() # only "(query-)continue" was retrieved. @@ -767,13 +858,18 @@ decide what to do with the contents of the dict. There will be one dict for each page queried via a titles= or ids= parameter (which must be supplied when instantiating this class). + + .. versionchanged:: 10.4 + Supports instance-level filtering via :attr:`filter_func + <APIGenerator.filter_func>` / :meth:`filter_item() + <APIGenerator.filter_item`. """

def __init__(self, prop: str, **kwargs) -> None: """Initializer.

- Required and optional parameters are as for ``Request``, except that - action=query is assumed and prop is required. + Required and optional parameters are as for ``Request``, except + that action=query is assumed and prop is required.

:param prop: the "prop=" type from api.php """ @@ -781,6 +877,7 @@ super().__init__(**kwargs) self._props = frozenset(prop.split('|')) self.resultkey = 'pages' + self._previous_dicts: dict[str, dict] = {}

@property def props(self): @@ -789,17 +886,28 @@

@property def generator(self): - """Yield results. + """Yield results from the API, including previously retrieved dicts.

.. versionchanged:: 7.6 - changed from iterator method to generator property + Changed from iterator method to generator property. + + .. versionchanged:: 10.4 + Items are filtered via :meth:`filter_item() + <APIGenerator.filter_item` inside :meth:`_extract_results`. + Previously retrieved dicts in `_previous_dicts` are also + filtered. + + :yield: Filtered page dicts """ - self._previous_dicts = {} + self._previous_dicts.clear() yield from super().generator yield from self._previous_dicts.values()

def _extract_results(self, resultdata): - """Yield completed page_data of consecutive API requests.""" + """Yield completed page_data of consecutive API requests. + + :meta public: + """ yield from self._fully_retrieved_data_dicts(resultdata) for data_dict in super()._extract_results(resultdata): if 'title' in data_dict: @@ -812,11 +920,20 @@ + str(data_dict))

def _fully_retrieved_data_dicts(self, resultdata): - """Yield items of self._previous_dicts that are not in resultdata.""" + """Yield items of self._previous_dicts that are not in resultdata. + + .. versionchanged:: 10.4 + Applies :meth:`filter_item()<APIGenerator.filter_item` to + previously stored dicts. + + :param resultdata: Current API response items + :yield: Filtered previously stored page dicts + """ resultdata_titles = {d['title'] for d in resultdata if 'title' in d} for prev_title, prev_dict in self._previous_dicts.copy().items(): if prev_title not in resultdata_titles: - yield prev_dict + if self.filter_item(prev_dict): + yield prev_dict del self._previous_dicts[prev_title]

@staticmethod diff --git a/pywikibot/site/_generators.py b/pywikibot/site/_generators.py index bf76ec7..2d4a7d1 100644 --- a/pywikibot/site/_generators.py +++ b/pywikibot/site/_generators.py @@ -26,7 +26,7 @@ ) from pywikibot.site._decorators import need_right from pywikibot.site._namespace import NamespaceArgType -from pywikibot.tools import deprecate_arg, is_ip_address +from pywikibot.tools import deprecate_arg, deprecate_positionals, is_ip_address from pywikibot.tools.itertools import filter_unique

@@ -925,9 +925,10 @@ for linkdata in pageitem['extlinks']: yield linkdata['*']

+ @deprecate_positionals(since='10.4.0') def allpages( self, - start: str = '!', + start: str = '!', *, prefix: str = '', namespace: SingleNamespaceType = 0, filterredir: bool | None = None, @@ -969,6 +970,11 @@ type such as bool, or an iterable with more than one namespace or *filterredir* parameter has an invalid type. """ + def _maxsize_filter(item): + """Return True if page text length is within maxsize limit.""" + return len(item.text.encode(self.encoding())) <= maxsize + + misermode = self.siteinfo.get('misermode') and maxsize is not None if filterredir not in (True, False, None): raise TypeError('filterredir parameter must be True, False or ' f'None, not {type(filterredir)}') @@ -976,7 +982,7 @@ apgen = self._generator(api.PageGenerator, type_arg='allpages', namespaces=namespace, gapfrom=start, total=total, - g_content=content) + g_content=content or misermode) if prefix: apgen.request['gapprefix'] = prefix if filterredir is not None: @@ -988,7 +994,7 @@ 'withoutlanglinks') if isinstance(minsize, int): apgen.request['gapminsize'] = str(minsize) - if isinstance(maxsize, int): + if not misermode and isinstance(maxsize, int): apgen.request['gapmaxsize'] = str(maxsize) if isinstance(protect_type, str): apgen.request['gapprtype'] = protect_type @@ -996,6 +1002,9 @@ apgen.request['gapprlevel'] = protect_level if reverse: apgen.request['gapdir'] = 'descending' + if misermode: + apgen.filter_func = _maxsize_filter + return apgen

def alllinks( diff --git a/tests/site_generators_tests.py b/tests/site_generators_tests.py index 7fbf716..f395c70 100755 --- a/tests/site_generators_tests.py +++ b/tests/site_generators_tests.py @@ -307,20 +307,15 @@ def test_allpages_pagesize(self) -> None: """Test allpages with page maxsize parameter.""" mysite = self.get_site() + encoding = mysite.encoding() for page in mysite.allpages(minsize=100, total=5): self.assertIsInstance(page, pywikibot.Page) self.assertTrue(page.exists()) - self.assertGreaterEqual(len(page.text.encode(mysite.encoding())), - 100) + self.assertGreaterEqual(len(page.text.encode(encoding)), 100) for page in mysite.allpages(maxsize=200, total=5): self.assertIsInstance(page, pywikibot.Page) self.assertTrue(page.exists()) - if len(page.text.encode(mysite.encoding())) > 200 \ - and mysite.data_repository() == mysite: # pragma: no cover - unittest_print( - f'{page}.text is > 200 bytes while raw JSON is <= 200') - continue - self.assertLessEqual(len(page.text.encode(mysite.encoding())), 200) + self.assertLessEqual(len(page.text.encode(encoding)), 200)

def test_allpages_protection(self) -> None: """Test allpages with protect_type parameter."""