jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/489905 )

Change subject: api.py: check if QueryGenerator support namespaces (updated) ......................................................................

api.py: check if QueryGenerator support namespaces (updated)

Check if 'query+module' supports namespace parameter and set namespace filtering accordingly in pagagenerators.

support_namespace() has been added for this purpose.

Note: - issue FutureWarning to alert that set_namespace() will soon raise a TypeError (and support_namespace() will then be removed). - this will be a breaking change, so time is given to fix 3rd party-code. - TODO left, so it will also be clear future evolution.

set_namespace() now returns a bool (it makes tests easier). This should not be an issue as now nothing is returned anyhow.

Tests added for set_namespace() and support_namespace().

Bug: T198452 Change-Id: Ia2c548cb5a6a8311c4bb92d3d345d6f16df6f7b1 --- M pywikibot/data/api.py M pywikibot/pagegenerators.py M tests/api_tests.py M tests/utils.py 4 files changed, 127 insertions(+), 18 deletions(-)

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

diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py index 1b86cf5..16ded04 100644 --- a/pywikibot/data/api.py +++ b/pywikibot/data/api.py @@ -2811,6 +2811,21 @@ self.api_limit), _logger)

+ def support_namespace(self): + """Check if namespace is a supported parameter on this query. + + Note: this function will be removed when self.set_namespace() will + throw TypeError() instead of just giving a warning. + See T196619. + + @return: True if yes, False otherwise + @rtype: bool + """ + assert(self.limited_module) # some modules do not have a prefix + return bool( + self.site._paraminfo.parameter('query+' + self.limited_module, + 'namespace')) + def set_namespace(self, namespaces): """Set a namespace filter on this query.

@@ -2820,9 +2835,12 @@ list of namespace identifiers. An empty iterator clears any namespace restriction. @raises KeyError: a namespace identifier was not resolved - @raises TypeError: a namespace identifier has an inappropriate - type such as NoneType or bool, or more than one namespace - if the API module does not support multiple namespaces + + # TODO: T196619 + # @raises TypeError: module does not support a namespace parameter + # or a namespace identifier has an inappropriate + # type such as NoneType or bool, or more than one namespace + # if the API module does not support multiple namespaces """ assert(self.limited_module) # some modules do not have a prefix param = self.site._paraminfo.parameter('query+' + self.limited_module, @@ -2830,7 +2848,16 @@ if not param: pywikibot.warning('{0} module does not support a namespace ' 'parameter'.format(self.limited_module)) - return + warn('set_namespace() will be modified to raise TypeError ' + 'when namespace parameter is not supported. ' + 'It will be a Breaking Change, please update your code ' + 'ASAP, due date July, 31st 2019.', FutureWarning, 2) + + # TODO: T196619 + # raise TypeError('{0} module does not support a namespace ' + # 'parameter'.format(self.limited_module)) + + return False

if isinstance(namespaces, basestring): namespaces = namespaces.split('|') @@ -2852,6 +2879,8 @@ elif self.prefix + 'namespace' in self.request: del self.request[self.prefix + 'namespace']

+ return None + def _query_continue(self): if all(key not in self.data[self.continue_name] for key in self.continuekey): diff --git a/pywikibot/pagegenerators.py b/pywikibot/pagegenerators.py index 9555c39..9a31c29 100644 --- a/pywikibot/pagegenerators.py +++ b/pywikibot/pagegenerators.py @@ -14,7 +14,7 @@ &params; """ # -# (C) Pywikibot team, 2008-2018 +# (C) Pywikibot team, 2008-2019 # # Distributed under the terms of the MIT license. # @@ -503,16 +503,19 @@ self.gens.insert(0, gen)

for i in range(len(self.gens)): - if isinstance(self.gens[i], pywikibot.data.api.QueryGenerator): - if self.namespaces: + if self.namespaces: + if (isinstance(self.gens[i], pywikibot.data.api.QueryGenerator) + and self.gens[i].support_namespace()): self.gens[i].set_namespace(self.namespaces) - if self.limit: - self.gens[i].set_maximum_items(self.limit) - else: - if self.namespaces: + # QueryGenerator does not support namespace param. + else: self.gens[i] = NamespaceFilterPageGenerator( self.gens[i], self.namespaces, self.site) - if self.limit: + + if self.limit: + try: + self.gens[i].set_maximum_items(self.limit) + except AttributeError: self.gens[i] = itertools.islice(self.gens[i], self.limit) if len(self.gens) == 0: if (self.titlefilter_list @@ -1013,8 +1016,7 @@

def _handle_unconnectedpages(self, value): """Handle `-unconnectedpages` argument.""" - # T196619 don't use QueryGenerator due to namespace filtering - return (p for p in self.site.unconnected_pages(total=_int_none(value))) + return self.site.unconnected_pages(total=_int_none(value))

def _handle_imagesused(self, value): """Handle `-imagesused` argument.""" diff --git a/tests/api_tests.py b/tests/api_tests.py index 76d5fc3..e4d71c1 100644 --- a/tests/api_tests.py +++ b/tests/api_tests.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """API test module.""" # -# (C) Pywikibot team, 2007-2018 +# (C) Pywikibot team, 2007-2019 # # Distributed under the terms of the MIT license. # @@ -852,6 +852,84 @@ self.assertEqual(len(links), count)

+class TestDryQueryGeneratorNamespaceParam(TestCase): + + """Test setting of namespace param with ListGenerator. + + Generators with different characteristics are used. + site._paraminfo is not always faithful to API, but serves the purpose + here. + """ + + family = 'wikipedia' + code = 'en' + + dry = True + + def setUp(self): + """Set up test case.""" + super(TestDryQueryGeneratorNamespaceParam, self).setUp() + self.site = self.get_site() + self.site._paraminfo['query+querypage'] = { + 'prefix': 'qp', + 'limit': {'max': 10}, + } + self.site._paraminfo['query+allpages'] = { + 'prefix': 'ap', + 'limit': {'max': 10}, + 'namespace': {'multi': True} + } + self.site._paraminfo['query+alllinks'] = { + 'prefix': 'al', + 'limit': {'max': 10}, + 'namespace': {'default': 0} + } + self.site._paraminfo['query+links'] = { + 'prefix': 'pl', + } + self.site._paraminfo.query_modules_with_limits = {'querypage', + 'allpages', + 'alllinks'} + + def test_namespace_for_module_with_no_limit(self): + """Test PageGenerator set_namespace.""" + self.gen = api.PageGenerator(site=self.site, + generator='links', + parameters={'titles': 'test'}) + self.assertRaises(AssertionError, self.gen.set_namespace, 0) + self.assertRaises(AssertionError, self.gen.set_namespace, 1) + self.assertRaises(AssertionError, self.gen.set_namespace, None) + + def test_namespace_param_is_not_settable(self): + """Test ListGenerator support_namespace.""" + self.gen = api.ListGenerator(listaction='querypage', site=self.site) + self.assertFalse(self.gen.support_namespace()) + self.assertFalse(self.gen.set_namespace([0, 1])) + + def test_namespace_none(self): + """Test ListGenerator set_namespace with None.""" + self.gen = api.ListGenerator(listaction='alllinks', site=self.site) + self.assertRaises(TypeError, self.gen.set_namespace, None) + + def test_namespace_non_multi(self): + """Test ListGenerator set_namespace when non multi.""" + self.gen = api.ListGenerator(listaction='alllinks', site=self.site) + self.assertRaises(TypeError, self.gen.set_namespace, [0, 1]) + self.assertIsNone(self.gen.set_namespace(0)) + + def test_namespace_multi(self): + """Test ListGenerator set_namespace when multi.""" + self.gen = api.ListGenerator(listaction='allpages', site=self.site) + self.assertTrue(self.gen.support_namespace()) + self.assertIsNone(self.gen.set_namespace([0, 1])) + + def test_namespace_resolve_failed(self): + """Test ListGenerator set_namespace when resolve fails.""" + self.gen = api.ListGenerator(listaction='allpages', site=self.site) + self.assertTrue(self.gen.support_namespace()) + self.assertRaises(KeyError, self.gen.set_namespace, 10000) + + class TestDryListGenerator(TestCase):

"""Test ListGenerator.""" @@ -879,7 +957,7 @@

def test_namespace_zero(self): """Test ListGenerator set_namespace with 0.""" - self.gen.set_namespace(0) + self.assertIsNone(self.gen.set_namespace(0))

class TestCachedRequest(DefaultSiteTestCase): diff --git a/tests/utils.py b/tests/utils.py index c0ce35b..5819f8d 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """Test utilities.""" # -# (C) Pywikibot team, 2013-2018 +# (C) Pywikibot team, 2013-2019 # # Distributed under the terms of the MIT license. # @@ -317,7 +317,7 @@

def parameter(self, module, param_name): """Load dry data.""" - return self[module][param_name] + return self[module].get(param_name)

def __getitem__(self, name): """Return dry data or a dummy parameter block."""