jenkins-bot has submitted this change and it was merged.
Change subject: Do not ask for password if user does not exist
......................................................................
Do not ask for password if user does not exist
When a password is not in the password file, or the login
failed for an unknown reason, check the user exists using
API allusers before halting to request the user enter a
password for a user which may not exist.
This allows a graceful error when an invalid username
is configured for a site, instead of halting for user
input which may not be possible if automated process,
and will be useless anyway as the user does not exist.
This may occur if a username is set for all sites in a
family, if the user does not exist on all sites, which
can be a problem the user cant fix when a site in the
family is 'closed' preventing new account registrations.
Bug: T74674
Bug: T85786
Bug: T100802
Change-Id: Ibab1846d71bdd5c6c457630078996911ea94af9d
---
M pywikibot/login.py
M tests/README.rst
M tests/aspects.py
3 files changed, 39 insertions(+), 3 deletions(-)
Approvals:
John Vandenberg: Looks good to me, but someone else must approve
XZise: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/login.py b/pywikibot/login.py
index 70f3f31..b3c4d1d 100644
--- a/pywikibot/login.py
+++ b/pywikibot/login.py
@@ -18,6 +18,7 @@
from warnings import warn
import pywikibot
+
from pywikibot import config
from pywikibot.tools import deprecated_args, normalize_username
from pywikibot.exceptions import NoUsername
@@ -102,6 +103,28 @@
self.password = password
if getattr(config, 'password_file', ''):
self.readPassword()
+
+ def check_user_exists(self):
+ """
+ Check that the username exists on the site.
+
+ @raises NoUsername: Username doesnt exist in user list.
+ """
+ try:
+ data = self.site.allusers(start=self.username, total=1)
+ user = next(iter(data))
+ except pywikibot.data.api.APIError as e:
+ if e.code == 'readapidenied':
+ pywikibot.warning('Could not check user %s exists on %s'
+ % (self.username, self.site))
+ return
+ else:
+ raise
+
+ if user['name'] != self.username:
+ # Report the same error as server error code NotExists
+ raise NoUsername('Username \'%s\' is invalid on %s'
+ % (self.username, self.site))
def botAllowed(self):
"""
@@ -224,6 +247,10 @@
@raises NoUsername: Username is not recognised by the site.
"""
if not self.password:
+ # First check that the username exists,
+ # to avoid asking for a password that will not work.
+ self.check_user_exists()
+
# As we don't want the password to appear on the screen, we set
# password = True
self.password = pywikibot.input(
diff --git a/tests/README.rst b/tests/README.rst
index 3080680..034f3b9 100644
--- a/tests/README.rst
+++ b/tests/README.rst
@@ -108,6 +108,9 @@
the password does not leak into the build logs.
4. The next build should run tests that require a logged in user
+If the username does not exist on one of the Travis build sites, user tests
+will not be run on that build site.
+
While passwords in travis-ci environment variables are not leaked in normal
operations, you are responsible for your own passwords.
diff --git a/tests/aspects.py b/tests/aspects.py
index ae29255..e52031a 100644
--- a/tests/aspects.py
+++ b/tests/aspects.py
@@ -41,7 +41,10 @@
import pywikibot
-from pywikibot import config, log, ServerError, Site
+import pywikibot.config2 as config
+
+from pywikibot import log, Site
+from pywikibot.exceptions import ServerError, NoUsername
from pywikibot.site import BaseSite
from pywikibot.family import WikimediaFamily
from pywikibot.comms import http
@@ -586,11 +589,14 @@
for site in cls.sites.values():
cls.require_site_user(site['family'], site['code'], sysop)
- site['site'].login(sysop)
+ try:
+ site['site'].login(sysop)
+ except NoUsername:
+ pass
if not site['site'].user():
raise unittest.SkipTest(
- '%s: Unable able to login to %s as %s'
+ '%s: Not able to login to %s as %s'
% (cls.__name__,
'sysop' if sysop else 'bot',
site['site']))
--
To view, visit https://gerrit.wikimedia.org/r/214816
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibab1846d71bdd5c6c457630078996911ea94af9d
Gerrit-PatchSet: 5
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: John Vandenberg <jayvdb(a)gmail.com>
Gerrit-Reviewer: John Vandenberg <jayvdb(a)gmail.com>
Gerrit-Reviewer: Ladsgroup <ladsgroup(a)gmail.com>
Gerrit-Reviewer: Merlijn van Deen <valhallasw(a)arctus.nl>
Gerrit-Reviewer: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: jenkins-bot <>
jenkins-bot has submitted this change and it was merged.
Change subject: [IMPROV] tests: General patcher for http module
......................................................................
[IMPROV] tests: General patcher for http module
Instead of just having a patcher for the request method in api_tests it moves a
more general patcher into utils to also patch fetch. The new patcher also
allows to intercept calls to request and fetch before and after the request has
been done. This will allow us to get the actual response from archive.org.
Bug: T104761
Change-Id: Icfbc14424ff61ddd31d2f346c54ccc3d11c714dc
---
M tests/api_tests.py
M tests/utils.py
M tests/weblib_tests.py
3 files changed, 137 insertions(+), 68 deletions(-)
Approvals:
John Vandenberg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/tests/api_tests.py b/tests/api_tests.py
index c3aa17f..c311922 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -10,10 +10,7 @@
__version__ = '$Id$'
import datetime
-import json
import types
-
-from collections import Mapping
import pywikibot.data.api as api
import pywikibot.family
@@ -29,72 +26,13 @@
DefaultSiteTestCase,
DefaultDrySiteTestCase,
)
-from tests.utils import allowed_failure, FakeLoginManager
+from tests.utils import allowed_failure, FakeLoginManager, PatchedHttp
if not PY2:
from urllib.parse import unquote_to_bytes
unicode = str
else:
from urllib import unquote_plus as unquote_to_bytes
-
-
-class PatchedRequest(object):
-
- """
- A ContextWrapper allowing Request to handle specific returned data.
-
- This patches the C{http} import in the L{pywikibot.data.api} module to a
- class simulating C{request}. It has a C{data} attribute which is either a
- static value which the requests will return or it's a callable returning the
- data. If it's a callable it'll be called with the same parameters as the
- original function in the L{pywikibot.comms.http} module, but with an extra
- argument C{parameters} which contains the extracted parameters.
-
- A unicode returned will be forwarded directly and a Mapping will be first
- converted into a json string. If it is False it'll use the original request
- and do an actual request. Any other types are not allowed.
- """
-
- class FakeHttp(object):
-
- """A fake http module to have a consistent response from request."""
-
- def __init__(self, wrapper):
- self.__wrapper = wrapper
-
- def request(self, *args, **kwargs):
- result = self.__wrapper.data
- if callable(result):
- result = result(*args, **kwargs)
- if result is False:
- return self.__wrapper._old_http.request(*args, **kwargs)
- elif isinstance(result, unicode):
- return result
- elif isinstance(result, Mapping):
- return json.dumps(result)
- else:
- raise ValueError('The result is not a valid type '
- '"{0}"'.format(type(result)))
-
- def __init__(self, data=None):
- """
- Initialize the context wrapper.
-
- @param data: The data for the request which may be changed later. It
- must be either unicode or Mapping before submitting a request.
- @type data: unicode or Mapping
- """
- super(PatchedRequest, self).__init__()
- self.data = data
-
- def __enter__(self):
- """Patch the http module property."""
- self._old_http = api.http
- api.http = PatchedRequest.FakeHttp(self)
-
- def __exit__(self, exc_type, exc_value, traceback):
- """Reset the http module property."""
- api.http = self._old_http
class TestAPIMWException(DefaultSiteTestCase):
@@ -138,7 +76,7 @@
"""Test a static request."""
req = api.Request(site=self.site, parameters={'action': 'query',
'fake': True})
- with PatchedRequest(self.data):
+ with PatchedHttp(api, self.data):
self.assertRaises(api.APIMWException, req.submit)
def test_API_error_encoding_ASCII(self):
@@ -148,7 +86,7 @@
'fake': True,
'titles': page})
self.assert_parameters = {'fake': ''}
- with PatchedRequest(self._dummy_request):
+ with PatchedHttp(api, self._dummy_request):
self.assertRaises(api.APIMWException, req.submit)
def test_API_error_encoding_Unicode(self):
@@ -158,7 +96,7 @@
'fake': True,
'titles': page})
self.assert_parameters = {'fake': ''}
- with PatchedRequest(self._dummy_request):
+ with PatchedHttp(api, self._dummy_request):
self.assertRaises(api.APIMWException, req.submit)
diff --git a/tests/utils.py b/tests/utils.py
index ac5fc50..4e06af0 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -8,6 +8,7 @@
from __future__ import print_function, unicode_literals
__version__ = '$Id$'
#
+import json
import os
import re
import subprocess
@@ -15,14 +16,18 @@
import time
import traceback
+from collections import Mapping
from warnings import warn
if sys.version_info[0] > 2:
import six
+ unicode = str
+
import pywikibot
from pywikibot import config
+from pywikibot.comms import threadedhttp
from pywikibot.site import Namespace
from pywikibot.data.api import CachedRequest
from pywikibot.data.api import Request as _original_Request
@@ -307,6 +312,121 @@
pass
+class DummyHttp(object):
+
+ """A class simulating the http module."""
+
+ def __init__(self, wrapper):
+ """Constructor with the given PatchedHttp instance."""
+ self.__wrapper = wrapper
+
+ def request(self, *args, **kwargs):
+ """The patched request method."""
+ result = self.__wrapper.before_request(*args, **kwargs)
+ if result is False:
+ result = self.__wrapper._old_http.request(*args, **kwargs)
+ elif isinstance(result, Mapping):
+ result = json.dumps(result)
+ elif not isinstance(result, unicode):
+ raise ValueError('The result is not a valid type '
+ '"{0}"'.format(type(result)))
+ response = self.__wrapper.after_request(result, *args, **kwargs)
+ if response is None:
+ response = result
+ return response
+
+ def fetch(self, *args, **kwargs):
+ """The patched fetch method."""
+ result = self.__wrapper.before_fetch(*args, **kwargs)
+ if result is False:
+ result = self.__wrapper._old_http.fetch(*args, **kwargs)
+ elif not isinstance(result, threadedhttp.HttpRequest):
+ raise ValueError('The result is not a valid type '
+ '"{0}"'.format(type(result)))
+ response = self.__wrapper.after_fetch(result, *args, **kwargs)
+ if response is None:
+ response = result
+ return response
+
+
+class PatchedHttp(object):
+
+ """
+ A ContextWrapper to handle any data going through the http module.
+
+ This patches the C{http} import in the given module to a class simulating
+ C{request} and C{fetch}. It has a C{data} attribute which is either a
+ static value which the requests will return or it's a callable returning the
+ data. If it's a callable it'll be called with the same parameters as the
+ original function in the L{http} module. For fine grained control it's
+ possible to override/monkey patch the C{before_request} and C{before_fetch}
+ methods. By default they just return C{data} directory or call it if it's
+ callable.
+
+ Even though L{http.request} is calling L{http.fetch}, it won't call the
+ patched method.
+
+ The data returned for C{request} may either be C{False}, a C{unicode} or a
+ C{Mapping} which is converted into a json string. The data returned for
+ C{fetch} can only be C{False} or a L{threadedhttp.HttpRequest}. For both
+ variants any other types are not allowed and if it is False it'll use the
+ original method and do an actual request.
+
+ Afterwards it is always calling C{after_request} or C{after_fetch} with the
+ response and given arguments. That can return a different response too, but
+ can also return None so that the original response is forwarded.
+ """
+
+ def __init__(self, module, data=None):
+ """
+ Constructor.
+
+ @param module: The given module to patch. It must have the http module
+ imported as http.
+ @type module: Module
+ @param data: The data returned for any request or fetch.
+ @type data: callable or False (or other depending on request/fetch)
+ """
+ super(PatchedHttp, self).__init__()
+ self._module = module
+ self.data = data
+
+ def _handle_data(self, *args, **kwargs):
+ """Return the data after it may have been called."""
+ if self.data is None:
+ raise ValueError('No handler is defined.')
+ elif callable(self.data):
+ return self.data(*args, **kwargs)
+ else:
+ return self.data
+
+ def before_request(self, *args, **kwargs):
+ """Return the value which should is returned by request."""
+ return self._handle_data(*args, **kwargs)
+
+ def before_fetch(self, *args, **kwargs):
+ """Return the value which should is returned by fetch."""
+ return self._handle_data(*args, **kwargs)
+
+ def after_request(self, response, *args, **kwargs):
+ """Handle the response after request."""
+ pass
+
+ def after_fetch(self, response, *args, **kwargs):
+ """Handle the response after fetch."""
+ pass
+
+ def __enter__(self):
+ """Patch the http module property."""
+ self._old_http = self._module.http
+ self._module.http = DummyHttp(self)
+ return self
+
+ def __exit__(self, exc_type, exc_value, traceback):
+ """Reset the http module property."""
+ self._module.http = self._old_http
+
+
def execute(command, data_in=None, timeout=0, error=None):
"""
Execute a command and capture outputs.
diff --git a/tests/weblib_tests.py b/tests/weblib_tests.py
index d05c853..18fa710 100644
--- a/tests/weblib_tests.py
+++ b/tests/weblib_tests.py
@@ -16,7 +16,9 @@
from urlparse import urlparse
import pywikibot.weblib as weblib
+
from tests.aspects import unittest, TestCase
+from tests.utils import PatchedHttp
class TestInternetArchive(TestCase):
@@ -29,15 +31,24 @@
},
}
+ def _test_response(self, response, *args, **kwargs):
+ # for later tests this is must be present, and it'll tell us the
+ # original content if that does not match
+ self.assertIn('closest', response.content)
+
def testInternetArchiveNewest(self):
- archivedversion = weblib.getInternetArchiveURL('https://google.com')
+ with PatchedHttp(weblib, False) as p:
+ p.after_fetch = self._test_response
+ archivedversion = weblib.getInternetArchiveURL('https://google.com')
parsed = urlparse(archivedversion)
self.assertIn(parsed.scheme, [u'http', u'https'])
self.assertEqual(parsed.netloc, u'web.archive.org')
self.assertTrue(parsed.path.strip('/').endswith('www.google.com'), parsed.path)
def testInternetArchiveOlder(self):
- archivedversion = weblib.getInternetArchiveURL('https://google.com', '200606')
+ with PatchedHttp(weblib, False) as p:
+ p.after_fetch = self._test_response
+ archivedversion = weblib.getInternetArchiveURL('https://google.com', '200606')
parsed = urlparse(archivedversion)
self.assertIn(parsed.scheme, [u'http', u'https'])
self.assertEqual(parsed.netloc, u'web.archive.org')
--
To view, visit https://gerrit.wikimedia.org/r/224644
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icfbc14424ff61ddd31d2f346c54ccc3d11c714dc
Gerrit-PatchSet: 2
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: John Vandenberg <jayvdb(a)gmail.com>
Gerrit-Reviewer: Ladsgroup <ladsgroup(a)gmail.com>
Gerrit-Reviewer: Merlijn van Deen <valhallasw(a)arctus.nl>
Gerrit-Reviewer: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: jenkins-bot <>
jenkins-bot has submitted this change and it was merged.
Change subject: [FIX] ParamInfo: Only return action and query mods
......................................................................
[FIX] ParamInfo: Only return action and query mods
The ParamInfo.modules property before 6e1f3918 returned just action and query
modules. This is reverting the change in 6e1f3918 to that previous state.
Change-Id: Iaa0c3c3eccc17d11abb6366de12e650bea9fa992
---
M pywikibot/data/api.py
M tests/api_tests.py
2 files changed, 10 insertions(+), 2 deletions(-)
Approvals:
John Vandenberg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py
index c0731eb..88701de 100644
--- a/pywikibot/data/api.py
+++ b/pywikibot/data/api.py
@@ -883,7 +883,7 @@
@deprecated('submodules() or module_paths')
def modules(self):
"""
- Set of all module names without path prefixes.
+ Set of all main and query modules without path prefixes.
Modules with the same names will be only added once (e.g. 'tokens' from
the action modules and query modules).
@@ -891,7 +891,7 @@
@return: module names
@rtype: set of str
"""
- return self._module_set(False)
+ return self.action_modules | self.query_modules
@property
def module_paths(self):
diff --git a/tests/api_tests.py b/tests/api_tests.py
index 9da7fb3..fdff58a 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -482,8 +482,16 @@
self.assertNotIn('flow', pi._modules)
pi.fetch(['flow'])
self.assertIn('flow', pi._modules)
+ other_modules = set()
for modules in pi._modules.values():
self.assertIsInstance(modules, frozenset)
+ other_modules |= modules
+
+ other_modules -= pi.action_modules
+ other_modules -= pi.query_modules
+ self.assertLessEqual(other_modules & pi.submodules('flow'),
+ pi.submodules('flow'))
+ self.assertFalse(other_modules & pi.modules)
class TestOptionSet(TestCase):
--
To view, visit https://gerrit.wikimedia.org/r/224767
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa0c3c3eccc17d11abb6366de12e650bea9fa992
Gerrit-PatchSet: 1
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: John Vandenberg <jayvdb(a)gmail.com>
Gerrit-Reviewer: Ladsgroup <ladsgroup(a)gmail.com>
Gerrit-Reviewer: Merlijn van Deen <valhallasw(a)arctus.nl>
Gerrit-Reviewer: jenkins-bot <>
jenkins-bot has submitted this change and it was merged.
Change subject: [FEAT] ParamInfo: Support any submodules
......................................................................
[FEAT] ParamInfo: Support any submodules
This automatically loads the submodules of fetched param info. This allows
ParamInfo to return all modules available and the submodules for a specific
module instead of just specifically query's and main's modules. With that it is
also possible to get the attributes in general for all modules, which is
necessary to get the prefixes. It deprecates some methods because they were
using the name and not the path.
It also enables users to determine to which module any of the parameters belong
which is currently only possible for submodules of the main and query modules.
Change-Id: I7f5438d8d98649405c9510a610743e4ce4fa7c82
---
M pywikibot/data/api.py
M tests/api_tests.py
M tests/dry_api_tests.py
3 files changed, 316 insertions(+), 47 deletions(-)
Approvals:
John Vandenberg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py
index c6cbf67..f54fa6b 100644
--- a/pywikibot/data/api.py
+++ b/pywikibot/data/api.py
@@ -145,6 +145,8 @@
Partially supports MW 1.11, using data extracted from API 'help'.
MW 1.10 not supported as module prefixes are not extracted from API 'help'.
+ It does not support the format modules.
+
TODO: Rewrite help parser to support earlier releases.
TODO: establish a data structure in the class which prefills
@@ -185,10 +187,11 @@
# Cached data.
self._prefixes = {}
+ self._prefix_map = {}
self._with_limits = None
- self._action_modules = None
- self._query_modules = [] # filled in _init()
+ self._action_modules = frozenset() # top level modules
+ self._modules = {} # filled in _init() (and enlarged in fetch)
self._limit = None
self.preloaded_modules = self.init_modules
@@ -199,6 +202,24 @@
self.modules_only_mode = modules_only_mode
if self.modules_only_mode:
self.paraminfo_keys = frozenset(['modules'])
+
+ def _add_submodules(self, name, modules):
+ """Add the modules to the internal cache or check if equal."""
+ # The current implementation here doesn't support submodules inside of
+ # submodules, because that would require to fetch all modules when only
+ # the names of them were requested
+ assert '+' not in name
+ modules = frozenset(modules)
+ if name == 'main':
+ # The main module behaves differently as it has no prefix
+ if self._action_modules:
+ assert modules == self._action_modules
+ else:
+ self._action_modules = modules
+ elif name in self._modules:
+ assert modules == self._modules[name]
+ else:
+ self._modules[name] = modules
def _init(self):
_mw_ver = MediaWikiVersion(self.site.version())
@@ -228,7 +249,7 @@
assert(main_modules_param)
assert('type' in main_modules_param)
assert(isinstance(main_modules_param['type'], list))
- self._action_modules = frozenset(main_modules_param['type'])
+ assert self._action_modules == set(main_modules_param['type'])
# While deprecated with warning in 1.25, paraminfo param 'querymodules'
# provides a list of all query modules. This will likely be removed
@@ -242,36 +263,14 @@
if query_modules_param and 'type' in query_modules_param:
# 1.19+ 'type' is the list of modules; on 1.18, it is 'string'
if isinstance(query_modules_param['type'], list):
- self._query_modules = frozenset(query_modules_param['type'])
+ self._add_submodules('query', query_modules_param['type'])
- if not self._query_modules:
- if 'query' not in self._paraminfo:
- self.fetch(set(['query']), _init=True)
+ if 'query' not in self._modules:
+ assert 'query' not in self._paraminfo
+ self.fetch(set(['query']), _init=True)
+ assert 'query' in self._modules
- meta_param = self.parameter('query', 'meta')
- prop_param = self.parameter('query', 'prop')
- list_param = self.parameter('query', 'list')
- generator_param = self.parameter('query', 'generator')
-
- assert(meta_param)
- assert(prop_param)
- assert(list_param)
- assert(generator_param)
- assert('type' in meta_param)
- assert('type' in prop_param)
- assert('type' in list_param)
- assert('type' in generator_param)
- assert(isinstance(meta_param['type'], list))
- assert(isinstance(prop_param['type'], list))
- assert(isinstance(list_param['type'], list))
- assert(isinstance(generator_param['type'], list))
-
- self._query_modules = frozenset(
- meta_param['type'] + prop_param['type'] + list_param['type'] +
- generator_param['type']
- )
-
- _reused_module_names = self._action_modules & self._query_modules
+ _reused_module_names = self._action_modules & self._modules['query']
# The only name clash in core between actions and query submodules is
# action=tokens and actions=query&meta=tokens, and this will warn if
@@ -323,6 +322,12 @@
action_modules = help_text[start:end].split(', ')
+ start = help_text.find('The format of the output')
+ start = help_text.find('One value: ', start) + len('One value: ')
+ end = help_text.find('\n', start)
+
+ format_modules = help_text[start:end].split(', ')
+
self._paraminfo['main'] = {
'name': 'main',
'path': 'main',
@@ -334,6 +339,12 @@
{
"name": "action",
'type': action_modules,
+ 'submodules': '',
+ },
+ {
+ 'name': 'format',
+ 'type': format_modules,
+ 'submodules': '',
},
],
}
@@ -399,14 +410,17 @@
{
'name': 'prop',
'type': prop_modules,
+ 'submodules': '',
},
{
'name': 'list',
'type': list_modules,
+ 'submodules': '',
},
{
'name': 'meta',
'type': meta_modules,
+ 'submodules': '',
},
{
'name': 'generator',
@@ -414,6 +428,8 @@
},
],
}
+ # Converted entries added, now treat them like they have been fetched
+ self._generate_submodules(['main', 'query'])
# TODO: rewrite 'help' parser to determine prefix from the parameter
# names, as API 1.10 help does not include prefix on the first line.
@@ -510,6 +526,15 @@
@type modules: set
@rtype: NoneType
"""
+ def module_generator():
+ """A generator yielding batches of modules."""
+ i = itergroup(sorted(modules), self._limit)
+ for batch in i:
+ for failed_module in failed_modules:
+ yield [failed_module]
+ del failed_modules[:]
+ yield batch
+
# The first request should be 'paraminfo', so that
# query modules can be prefixed with 'query+'
# If _init is True, dont call _init().
@@ -523,7 +548,7 @@
if not modules:
return
- assert(self._query_modules or _init)
+ assert 'query' in self._modules or _init
if MediaWikiVersion(self.site.version()) < MediaWikiVersion("1.12"):
# When the help is parsed, all paraminfo should already be loaded
@@ -532,6 +557,11 @@
% modules, _logger=_logger)
return
+ # If something went wrong in a batch it can add each module to the
+ # batch and the generator will on the next iteration yield each module
+ # separately
+ failed_modules = []
+
# This can be further optimised, by grouping them in more stable
# subsets, which are unlikely to change. i.e. first request core
# modules which have been a stable part of the API for a long time.
@@ -539,7 +569,7 @@
# Also, when self.modules_only_mode is disabled, both modules and
# querymodules may each be filled with self._limit items, doubling the
# number of modules that may be processed in a single batch.
- for module_batch in itergroup(sorted(modules), self._limit):
+ for module_batch in module_generator():
if self.modules_only_mode and 'pageset' in module_batch:
pywikibot.debug('paraminfo fetch: removed pageset', _logger)
module_batch.remove('pageset')
@@ -575,11 +605,99 @@
result = request.submit()
normalized_result = self.normalize_paraminfo(result)
+ for path in list(normalized_result):
+ if normalized_result[path] is False:
+ del normalized_result[path]
+
+ # Sometimes the name/path of the module is not actually the name
+ # which was requested, so we need to manually determine which
+ # (wrongly named) module uses which actual name. See also T105478
+ missing_modules = [m for m in module_batch
+ if m not in normalized_result]
+ if len(missing_modules) == 1 and len(normalized_result) == 1:
+ # Okay it's possible to recover
+ normalized_result = next(iter(normalized_result.values()))
+ pywikibot.warning('The module "{0[name]}" ("{0[path]}") '
+ 'was returned as path even though "{1}" '
+ 'was requested'.format(normalized_result,
+ missing_modules[0]))
+ normalized_result['path'] = missing_modules[0]
+ normalized_result['name'] = missing_modules[0].rsplit('+')[0]
+ normalized_result = {missing_modules[0]: normalized_result}
+ elif len(module_batch) > 1 and missing_modules:
+ # Rerequest the missing ones separately
+ pywikibot.log('Inconsitency in batch "{0}"; rerequest '
+ 'separately'.format(missing_modules))
+ failed_modules.extend(missing_modules)
+
+ # Remove all modules which weren't requested, we can't be sure that
+ # they are valid
+ for path in list(normalized_result):
+ if path not in module_batch:
+ del normalized_result[path]
self._paraminfo.update(normalized_result)
+ self._generate_submodules(mod['path']
+ for mod in normalized_result.values())
if 'pageset' in modules and 'pageset' not in self._paraminfo:
self._emulate_pageset()
+
+ def _generate_submodules(self, modules):
+ """Check and generate submodules for the given modules."""
+ for module in modules:
+ parameters = self._paraminfo[module].get('parameters', [])
+ submodules = set()
+ # Advanced submodule into added to MW API in df80f1ea
+ if self.site.version() >= MediaWikiVersion('1.26wmf9'):
+ # This is supplying submodules even if they aren't submodules of
+ # the given module so skip those
+ for param in parameters:
+ if ((module == 'main' and param['name'] == 'format') or
+ 'submodules' not in param):
+ continue
+ for submodule in param['submodules'].values():
+ if '+' in submodule:
+ parent, child = submodule.rsplit('+', 1)
+ else:
+ parent = 'main'
+ child = submodule
+ if parent == module:
+ submodules.add(child)
+ else:
+ # Boolean submodule info added to MW API in afa153ae
+ if self.site.version() >= MediaWikiVersion('1.24wmf18'):
+ params = set(param['name'] for param in parameters
+ if 'submodules' in param)
+ else:
+ if module == 'main':
+ params = set(['action'])
+ elif module == 'query':
+ params = set(['prop', 'list', 'meta'])
+ else:
+ params = set()
+ for param in parameters:
+ if param['name'] in params:
+ param['submodules'] = ''
+
+ for param in params:
+ param = self.parameter(module, param)
+ # Do not add format modules
+ if module != 'main' or param['name'] != 'format':
+ submodules |= set(param['type'])
+
+ if submodules:
+ self._add_submodules(module, submodules)
+ if module == 'query':
+ # Previously also modules from generator were used as query
+ # modules, but verify that those are just a subset of the
+ # prop/list/meta modules. There is no sanity check as this
+ # needs to be revisited if query has no generator parameter
+ for param in parameters:
+ if param['name'] == 'generator':
+ break
+ assert param['name'] == 'generator' and \
+ submodules >= set(param['type'])
def _normalize_modules(self, modules):
"""Add query+ to any query module name not also in action modules."""
@@ -590,7 +708,7 @@
assert(self._action_modules)
return set('query+' + mod if '+' not in mod and
- mod in self._query_modules and
+ mod in self.query_modules and
mod not in self._action_modules
else mod
for mod in modules)
@@ -610,7 +728,11 @@
@classmethod
def normalize_paraminfo(cls, data):
- """Convert both old and new API JSON into a new-ish data structure."""
+ """
+ Convert both old and new API JSON into a new-ish data structure.
+
+ For duplicate paths, the value will be False.
+ """
result_data = {}
for paraminfo_key, modules_data in data['paraminfo'].items():
if not modules_data:
@@ -652,7 +774,15 @@
path = mod_data['path']
- result_data[path] = mod_data
+ if path in result_data:
+ # Only warn first time
+ if result_data[path] is not False:
+ pywikibot.warning('Path "{0}" is ambiguous.'.format(path))
+ else:
+ pywikibot.log('Found another path "{0}"'.format(path))
+ result_data[path] = False
+ else:
+ result_data[path] = mod_data
return result_data
@@ -730,19 +860,37 @@
return param_data
@property
+ @deprecated('submodules() or module_paths')
def modules(self):
"""
Set of all module names without path prefixes.
- Only includes one 'tokens', even if it appears as both a
- action and a query submodule.
+ Modules with the same names will be only added once (e.g. 'tokens' from
+ the action modules and query modules).
@return: module names
@rtype: set of str
"""
+ return self._module_set(False)
+
+ @property
+ def module_paths(self):
+ """Set of all modules using their paths."""
+ return self._module_set(True)
+
+ # As soon as modules() is removed, module_paths and _module_set can be
+ # combined, so don't add any code between these two methods.
+ def _module_set(self, path):
if not self.__inited:
self._init()
- return self._action_modules | self._query_modules
+ # Load the submodules of all action modules available
+ self.fetch(self.action_modules)
+ modules = set(self.action_modules)
+ for parent_module in self._modules:
+ submodules = self.submodules(parent_module, path)
+ assert not submodules & modules or not path
+ modules |= submodules
+ return modules
@property
def action_modules(self):
@@ -754,9 +902,27 @@
@property
def query_modules(self):
"""Set of all query module names without query+ path prefix."""
+ return self.submodules('query')
+
+ def submodules(self, name, path=False):
+ """
+ Set of all submodules.
+
+ @param name: The name of the parent module.
+ @type name: str
+ @param path: Whether the path and not the name is returned.
+ @type path: bool
+ @return: The names or paths of the submodules.
+ @rtype: set
+ """
if not self.__inited:
self._init()
- return self._query_modules
+ if name not in self._modules:
+ self.fetch([name])
+ submodules = self._modules[name]
+ if path:
+ submodules = self._prefix_submodules(submodules, name)
+ return submodules
@staticmethod
def _prefix_submodules(modules, prefix):
@@ -764,6 +930,7 @@
return set('{0}+{1}'.format(prefix, mod) for mod in modules)
@property
+ @deprecated('prefix_map')
def prefixes(self):
"""
Mapping of module to its prefix for all modules with a prefix.
@@ -774,19 +941,55 @@
self._prefixes = self.module_attribute_map('prefix')
return self._prefixes
+ @property
+ def prefix_map(self):
+ """
+ Mapping of module to its prefix for all modules with a prefix.
+
+ This loads paraminfo for all modules.
+ """
+ if not self._prefix_map:
+ self._prefix_map = dict((module, prefix) for module, prefix in
+ self.attributes('prefix').items()
+ if prefix)
+ return self._prefix_map.copy()
+
+ def attributes(self, attribute, modules=None):
+ """
+ Mapping of modules with an attribute to the attribute value.
+
+ It will include all modules which have that attribute set, also if that
+ attribute is empty or set to False.
+
+ @param attribute: attribute name
+ @type attribute: basestring
+ @param modules: modules to include. If None (default), it'll load all
+ modules including all submodules using the paths.
+ @type modules: set or None
+ @rtype: dict using modules as keys
+ """
+ if modules is None:
+ modules = self.module_paths
+ self.fetch(modules)
+
+ return dict((mod, self[mod][attribute])
+ for mod in modules
+ if attribute in self[mod])
+
+ @deprecated('attributes')
def module_attribute_map(self, attribute, modules=None):
"""
Mapping of modules with an attribute to the attribute value.
@param attribute: attribute name
@type attribute: basestring
- @param modules: modules to include (default: all modules)
+ @param modules: modules to include. If None (default) it'll load all
+ action and query modules using the module names. It only uses the
+ path for query modules which have the same name as an action module.
@type modules: set
@rtype: dict using modules as keys
"""
if modules is None:
- # TODO: The keys for modules with a clash are path prefixed
- # which is different from all other keys.
modules = self.modules | self._prefix_submodules(
self.query_modules & self.action_modules, 'query')
@@ -800,7 +1003,7 @@
def query_modules_with_limits(self):
"""Set of all query modules which have limits."""
if not self._with_limits:
- self.fetch(self._prefix_submodules(self.query_modules, 'query'))
+ self.fetch(self.submodules('query', True))
self._with_limits = frozenset(
[mod for mod in self.query_modules
if self.parameter('query+' + mod, 'limit')])
diff --git a/tests/api_tests.py b/tests/api_tests.py
index b0f88af..0b18871 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -76,7 +76,7 @@
self.assertEqual(len(pi),
len(pi.preloaded_modules))
- self.assertIn('info', pi._query_modules)
+ self.assertIn('info', pi.query_modules)
self.assertIn('login', pi._action_modules)
def test_init_pageset(self):
@@ -208,6 +208,7 @@
self.assertNotIn('foobar', pi._paraminfo)
self.assertRaises(KeyError, pi.__getitem__, 'foobar')
+ self.assertRaises(KeyError, pi.__getitem__, 'foobar+foobar')
self.assertIn('main', pi._paraminfo)
self.assertIn('paraminfo', pi._paraminfo)
@@ -217,6 +218,23 @@
self.assertEqual(len(pi),
len(pi.preloaded_modules))
+
+ def test_submodules(self):
+ """Test another module apart from query having submodules."""
+ pi = api.ParamInfo(self.site)
+ self.assertFalse(pi._modules)
+ pi.fetch(['query'])
+ self.assertIn('query', pi._modules)
+ self.assertIsInstance(pi._modules['query'], frozenset)
+ self.assertIn('revisions', pi._modules['query'])
+ self.assertEqual(pi.submodules('query'), pi.query_modules)
+ for mod in pi.submodules('query', True):
+ self.assertEqual(mod[:6], 'query+')
+ self.assertEqual(mod[6:], pi[mod]['name'])
+ self.assertEqual(mod, pi[mod]['path'])
+
+ self.assertRaises(KeyError, pi.__getitem__, 'query+foobar')
+ self.assertRaises(KeyError, pi.submodules, 'edit')
def test_query_modules_with_limits(self):
site = self.get_site()
@@ -231,6 +249,17 @@
self.assertIn('revisions', pi.modules)
self.assertIn('help', pi.modules)
self.assertIn('allpages', pi.modules)
+ for mod in pi.modules:
+ self.assertNotIn('+', mod)
+
+ def test_module_paths(self):
+ """Test module paths use the complete paths."""
+ pi = api.ParamInfo(self.site)
+ self.assertIn('help', pi.module_paths)
+ self.assertNotIn('revisions', pi.module_paths)
+ self.assertIn('query+revisions', pi.module_paths)
+ self.assertNotIn('allpages', pi.module_paths)
+ self.assertIn('query+allpages', pi.module_paths)
def test_prefixes(self):
"""Test v1.8 module prefixes exist."""
@@ -239,6 +268,24 @@
self.assertIn('revisions', pi.prefixes)
self.assertIn('login', pi.prefixes)
self.assertIn('allpages', pi.prefixes)
+
+ def test_prefix_map(self):
+ """Test module prefixes use the path."""
+ pi = api.ParamInfo(self.site)
+ self.assertIn('query+revisions', pi.prefix_map)
+ self.assertIn('login', pi.prefix_map)
+ self.assertIn('query+allpages', pi.prefix_map)
+ for mod in pi.prefix_map:
+ self.assertEqual(mod, pi[mod]['path'])
+
+ def test_attributes(self):
+ """Test attributes method."""
+ pi = api.ParamInfo(self.site)
+ attributes = pi.attributes('mustbeposted')
+ self.assertIn('edit', attributes)
+ for mod, value in attributes.items():
+ self.assertEqual(mod, pi[mod]['path'])
+ self.assertEqual(value, '')
def test_old_mode(self):
site = self.get_site()
@@ -274,6 +321,25 @@
self.assertIn('revisions', pi.prefixes)
+class TestOtherSubmodule(TestCase):
+
+ """Test handling multiple different modules having submodules."""
+
+ family = 'mediawiki'
+ code = 'mediawiki'
+
+ def test_other_submodule(self):
+ """Test another module apart from query having submodules."""
+ pi = api.ParamInfo(self.site)
+ self.assertFalse(pi._modules)
+ pi.fetch(['query'])
+ self.assertNotIn('flow', pi._modules)
+ pi.fetch(['flow'])
+ self.assertIn('flow', pi._modules)
+ for modules in pi._modules.values():
+ self.assertIsInstance(modules, frozenset)
+
+
class TestOptionSet(TestCase):
"""OptionSet class test class."""
diff --git a/tests/dry_api_tests.py b/tests/dry_api_tests.py
index ab80178..d3232ad 100644
--- a/tests/dry_api_tests.py
+++ b/tests/dry_api_tests.py
@@ -332,8 +332,8 @@
# Pretend that paraminfo has been loaded
for mod in site._paraminfo.init_modules:
site._paraminfo._paraminfo[mod] = {}
- site._paraminfo._query_modules = ['info']
- site._paraminfo._action_modules = ['edit']
+ site._paraminfo._action_modules = frozenset(['edit'])
+ site._paraminfo._modules = {'query': frozenset(['info'])}
# TODO: remove access of this private member of ParamInfo
site._paraminfo._ParamInfo__inited = True
--
To view, visit https://gerrit.wikimedia.org/r/198247
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7f5438d8d98649405c9510a610743e4ce4fa7c82
Gerrit-PatchSet: 15
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: John Vandenberg <jayvdb(a)gmail.com>
Gerrit-Reviewer: Ladsgroup <ladsgroup(a)gmail.com>
Gerrit-Reviewer: Merlijn van Deen <valhallasw(a)arctus.nl>
Gerrit-Reviewer: Ricordisamoa <ricordisamoa(a)openmailbox.org>
Gerrit-Reviewer: XZise <CommodoreFabianus(a)gmx.de>
Gerrit-Reviewer: jenkins-bot <>