jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/341342 )

Change subject: [IMPR] rewrite Revision class ......................................................................

[IMPR] rewrite Revision class

site.py: - use a new method _rvprops to assign rvprop properties for loadrevisions() and preloadpages(). Minimum supported mw version is 1.19; remove other dependencies - enable all rvprop properties - rewrite loadrevisions

api.py: - instantiate the Revision class with the revision dictionary. All api results can be used and further api modifications would be much easier to implement in the central Revision class - page.title() is used to solve T102735 (content model for mw < 1.21)

page.py: - move Revision class to _revision.py

_revision.py - use a collection Mapping to hold the data. This disables changing the revision. - instantiate the Revision class with the revision dictionary via api. - for backup compatibility cleanup positional args but deprecate their usage; they shouldn't be used outside api.py - upcast some data like timestamp - take into account that slots are provided by some mw versions - deprecte parent_id and content_model properties - Remove private Revision._thank staticmethod which was used for thank tests only but is not necessary to have.

thanks_tests.py - use site.thank_revison method instead of Revision._thank

Bug: T102735 Bug: T259428 Change-Id: I6a87434aca7b83368a6fbf1742e1b10c7dbb3252 --- M pywikibot/CONTENT.rst M pywikibot/data/api.py M pywikibot/page/__init__.py A pywikibot/page/_revision.py M pywikibot/site/__init__.py M tests/thanks_tests.py 6 files changed, 233 insertions(+), 203 deletions(-)

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

diff --git a/pywikibot/CONTENT.rst b/pywikibot/CONTENT.rst index 30f92ee..3446ee4 100644 --- a/pywikibot/CONTENT.rst +++ b/pywikibot/CONTENT.rst @@ -104,6 +104,8 @@ +============================+======================================================+ | __init__.py | Objects representing MediaWiki pages | +----------------------------+------------------------------------------------------+ + | _revision.py | Object representing page revision | + +----------------------------+------------------------------------------------------+

+----------------------------+------------------------------------------------------+ diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py index 81175d4..19f93d8 100644 --- a/pywikibot/data/api.py +++ b/pywikibot/data/api.py @@ -3172,23 +3172,19 @@

def _update_revisions(page, revisions): """Update page revisions.""" - # TODO: T102735: Use the page content model for <1.21 + content_model = {'.js': 'javascript', '.css': 'css'} for rev in revisions: - revision = pywikibot.page.Revision( - revid=rev['revid'], - timestamp=pywikibot.Timestamp.fromISOformat(rev['timestamp']), - user=rev.get('user', ''), - anon='anon' in rev, - comment=rev.get('comment', ''), - minor='minor' in rev, - slots=rev.get('slots'), # MW 1.32+ - text=rev.get('*'), # b/c - rollbacktoken=rev.get('rollbacktoken'), - parentid=rev.get('parentid'), - contentmodel=rev.get('contentmodel'), # b/c - sha1=rev.get('sha1') - ) - page._revisions[revision.revid] = revision + if page.site.mw_version < '1.21': + # T102735: use content model depending on the page suffix + title = page.title(with_ns=False) + for suffix, cm in content_model.items(): + if title.endswith(suffix): + rev['contentmodel'] = cm + break + else: + rev['contentmodel'] = 'wikitext' + + page._revisions[rev['revid']] = pywikibot.page.Revision(**rev)

def _update_templates(page, templates): diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py index 5452121..9f1a402 100644 --- a/pywikibot/page/__init__.py +++ b/pywikibot/page/__init__.py @@ -16,7 +16,6 @@ # # Distributed under the terms of the MIT license. # -import hashlib import logging import os.path import re @@ -44,6 +43,7 @@ UserRightsError, ) from pywikibot.family import Family +from pywikibot.page._revision import Revision from pywikibot.site import DataSite, Namespace, need_version from pywikibot.tools import ( add_full_name, @@ -5484,139 +5484,6 @@ }

-class Revision(DotReadableDict): - - """A structure holding information about a single revision of a Page.""" - - def __init__(self, revid, timestamp, user, anon=False, comment='', - text=None, minor=False, rollbacktoken=None, parentid=None, - contentmodel=None, sha1=None, slots=None): - """ - Initializer. - - All parameters correspond to object attributes (e.g., revid - parameter is stored as self.revid) - - @param revid: Revision id number - @type revid: int - @param text: Revision wikitext. - @type text: str, or None if text not yet retrieved - @param timestamp: Revision time stamp - @type timestamp: pywikibot.Timestamp - @param user: user who edited this revision - @type user: str - @param anon: user is unregistered - @type anon: bool - @param comment: edit comment text - @type comment: str - @param minor: edit flagged as minor - @type minor: bool - @param rollbacktoken: rollback token - @type rollbacktoken: str - @param parentid: id of parent Revision - @type parentid: int - @param contentmodel: content model label (v1.21+) - @type contentmodel: str - @param sha1: sha1 of revision text - @type sha1: str - @param slots: revision slots (v1.32+) - @type slots: dict - """ - self.revid = revid - self._text = text - self.timestamp = timestamp - self.user = user - self.anon = anon - self.comment = comment - self.minor = minor - self.rollbacktoken = rollbacktoken - self._parent_id = parentid - self._content_model = contentmodel - self._sha1 = sha1 - self.slots = slots - - @property - def parent_id(self) -> int: - """ - Return id of parent/previous revision. - - Returns 0 if there is no previous revision - - @return: id of parent/previous revision - @raises AssertionError: parent id not supplied to the constructor - """ - assert self._parent_id is not None, ( - 'Revision {0} was instantiated without a parent id' - .format(self.revid)) - - return self._parent_id - - @property - def text(self) -> Optional[str]: - """ - Return text of this revision. - - This is meant for compatibility with older MW version which - didn't support revisions with slots. For newer MW versions, - this returns the contents of the main slot. - - @return: text of the revision - """ - if self.slots is not None: - return self.slots.get('main', {}).get('*') - return self._text - - @property - def content_model(self) -> str: - """ - Return content model of the revision. - - This is meant for compatibility with older MW version which - didn't support revisions with slots. For newer MW versions, - this returns the content model of the main slot. - - @return: content model - @raises AssertionError: content model not supplied to the constructor - which always occurs for MediaWiki versions lower than 1.21. - """ - if self._content_model is None and self.slots is not None: - self._content_model = self.slots.get('main', {}).get( - 'contentmodel') - # TODO: T102735: Add a sane default of 'wikitext' and others for <1.21 - assert self._content_model is not None, ( - 'Revision {0} was instantiated without a content model' - .format(self.revid)) - - return self._content_model - - @property - def sha1(self) -> Optional[str]: - """ - Return and cache SHA1 checksum of the text. - - @return: if the SHA1 checksum is cached it'll be returned which is the - case when it was requested from the API. Otherwise it'll use the - revision's text to calculate the checksum (encoding it using UTF8 - first). That calculated checksum will be cached too and returned on - future calls. If the text is None (not queried) it will just return - None and does not cache anything. - """ - if self._sha1 is None and self.text is not None: - self._sha1 = hashlib.sha1(self.text.encode('utf8')).hexdigest() - return self._sha1 - - @staticmethod - def _thank(revid, site, source='pywikibot'): - """Thank a user for this revision. - - @param site: The Site object for this revision. - @type site: Site - @param source: An optional source to pass to the API. - @type source: str - """ - site.thank_revision(revid, source) - - class FileInfo(DotReadableDict):

""" diff --git a/pywikibot/page/_revision.py b/pywikibot/page/_revision.py new file mode 100644 index 0000000..9331b46 --- /dev/null +++ b/pywikibot/page/_revision.py @@ -0,0 +1,154 @@ +# -*- coding: utf-8 -*- +"""Object representing page revision.""" +# +# (C) Pywikibot team, 2008-2020 +# +# Distributed under the terms of the MIT license. +# +import hashlib + +from collections.abc import Mapping + +from pywikibot import Timestamp, warning +from pywikibot.tools import deprecated, issue_deprecation_warning + + +class Revision(Mapping): + + """A structure holding information about a single revision of a Page. + + Each data item can be accessed either by its key or as an attribute + with the attribute name equal to the key e.g.: + + >>> r = Revision(comment='Sample for Revision access') + >>> r.comment == r['comment'] + True + >>> r.comment + Sample for Revision access + """ + + def __init__(self, *args, **kwargs): + """Initializer.""" + self._clean_args(args, kwargs) + self._data = kwargs + self._upcast_dict(self._data) + super().__init__() + + def _clean_args(self, args: tuple, kwargs: dict): + """Cleanup positional arguments. + + Replace positional arguments with keyword arguments for + backwards compatibility. + @param: args: tuple of positional arguments + @param: kwargs: mutable dict of keyword arguments to be updated + """ + keys = ( # postional argument keys by old order + 'revid', 'timestamp', 'user', 'anon', 'comment', 'text', 'minor', + 'rollbacktoken', 'parentid', 'contentmodel', 'sha1', 'slots' + ) + + # replace positional arguments with keyword arguments + for i, (arg, key) in enumerate(zip(args, keys)): + issue_deprecation_warning('Positional argument {} ({})' + .format(i + 1, arg), + 'keyword argument "{}={}"' + .format(key, arg), + warning_class=FutureWarning, + since='20200802') + if key in kwargs: + warning('"{}" is given as keyword argument "{}" already; ' + 'ignoring "{}"'.format(key, arg, kwargs[key])) + else: + kwargs[key] = arg + + @staticmethod + def _upcast_dict(map_): + """Upcast dictionary values.""" + map_['timestamp'] = Timestamp.fromISOformat(map_['timestamp']) + + map_.update(anon='anon' in map_) + map_.setdefault('comment', '') + map_.update(minor='minor' in map_) + + if 'slots' in map_: # mw 1.32+ + mainslot = map_['slots'].get('main', {}) + map_['text'] = mainslot.get('*') + map_['contentmodel'] = mainslot.get('contentmodel') + else: + map_['slots'] = None + map_['text'] = map_.get('*') + + map_.setdefault('sha1') + if map_['sha1'] is None and map_['text'] is not None: + map_['sha1'] = hashlib.sha1( + map_['text'].encode('utf8')).hexdigest() + + def __len__(self) -> int: + """Return the number of data items.""" + return len(self._data) + + def __getitem__(self, name: str): + """Return a single Revision item given by name.""" + if name in self._data: + return self._data[name] + if name in ('parent_id', 'content_model'): + return getattr(self, name) + + return self.__missing__(name) + + # provide attribute access + __getattr__ = __getitem__ + + def __iter__(self): + """Provide Revision data as iterator.""" + return iter(self._data) + + def __repr__(self): + """String representation of Revision.""" + return '{}({})'.format(self.__class__.__name__, self._data) + + def __str__(self): + """Printable representation of Revision data.""" + return str(self._data) + + def __missing__(self, key): + """Provide backward compatibility for exceptions.""" + if key == 'parentid': + raise AssertionError( + 'Revision {rev.revid} was instantiated without a parent id' + .format(rev=self)) + + if key == 'rollbacktoken': + warning('"rollbacktoken" has been deprecated since MediaWiki 1.24') + return None + + # raise AttributeError instead of KeyError for backward compatibility + raise AttributeError("'{}' object has no attribute '{}'" + .format(self.__class__.__name__, key)) + + @property + @deprecated('parentid property', since='20200802') + def parent_id(self) -> int: + """DEPRECATED. Return id of parent/previous revision. + + Returns 0 if there is no previous revision + + @return: id of parent/previous revision + @raises AssertionError: parent id not supplied to the constructor + """ + return self.parentid + + @property + @deprecated('contentmodel', since='20200802') + def content_model(self) -> str: + """DEPRECATED. Return content model of the revision. + + This is meant for compatibility with older MW version which + didn't support revisions with slots. For newer MW versions, + this returns the content model of the main slot. + + @return: content model + @raises AssertionError: content model not supplied to the constructor + which always occurs for MediaWiki versions lower than 1.21. + """ + return self.contentmodel diff --git a/pywikibot/site/__init__.py b/pywikibot/site/__init__.py index edfbf11..0ce5158 100644 --- a/pywikibot/site/__init__.py +++ b/pywikibot/site/__init__.py @@ -3089,8 +3089,6 @@ if pageprops: props += '|pageprops'

- rvprop = ['ids', 'flags', 'timestamp', 'user', 'comment', 'content'] - parameter = self._paraminfo.parameter('query+info', 'prop') if self.logged_in() and self.has_right('apihighlimits'): max_ids = int(parameter['highlimit']) @@ -3120,7 +3118,7 @@ rvgen.request['pageids'] = set(pageids) else: rvgen.request['titles'] = list(cache.keys()) - rvgen.request['rvprop'] = rvprop + rvgen.request['rvprop'] = self._rvprops(content=True) pywikibot.output('Retrieving %s pages from %s.' % (len(cache), self))

@@ -3724,12 +3722,24 @@ yield from self._generator(api.PageGenerator, namespaces=namespaces, total=total, g_content=content, **cmargs)

+ def _rvprops(self, content=False) -> list: + """Setup rvprop items for loadrevisions and preloadpages. + + @return: rvprop items + """ + props = ['comment', 'ids', 'flags', 'parsedcomment', 'sha1', 'size', + 'tags', 'timestamp', 'user', 'userid'] + if content: + props.append('content') + if self.mw_version >= '1.21': + props.append('contentmodel') + if self.mw_version >= '1.32': + props.append('roles') + return props + @deprecated_args(getText='content', sysop=None) @remove_last_args(['rollback']) - def loadrevisions(self, page, *, content=False, revids=None, - startid=None, endid=None, starttime=None, - endtime=None, rvdir=None, user=None, excludeuser=None, - section=None, step=None, total=None): + def loadrevisions(self, page, *, content=False, section=None, **kwargs): """Retrieve revision information and store it in page object.

By default, retrieves the last (current) revision of the page, @@ -3753,61 +3763,60 @@ (content must be True); section must be given by number (top of the article is section 0), not name @type section: int - @param revids: retrieve only the specified revision ids (raise + @keyword revids: retrieve only the specified revision ids (raise Exception if any of revids does not correspond to page) @type revids: an int, a str or a list of ints or strings - @param startid: retrieve revisions starting with this revid - @param endid: stop upon retrieving this revid - @param starttime: retrieve revisions starting at this Timestamp - @param endtime: stop upon reaching this Timestamp - @param rvdir: if false, retrieve newest revisions first (default); + @keyword startid: retrieve revisions starting with this revid + @keyword endid: stop upon retrieving this revid + @keyword starttime: retrieve revisions starting at this Timestamp + @keyword endtime: stop upon reaching this Timestamp + @keyword rvdir: if false, retrieve newest revisions first (default); if true, retrieve earliest first - @param user: retrieve only revisions authored by this user - @param excludeuser: retrieve all revisions not authored by this user + @keyword user: retrieve only revisions authored by this user + @keyword excludeuser: retrieve all revisions not authored by this user @raises ValueError: invalid startid/endid or starttime/endtime values @raises pywikibot.Error: revids belonging to a different page """ - latest = (revids is None - and startid is None - and endid is None - and starttime is None - and endtime is None - and rvdir is None - and user is None - and excludeuser is None - and step is None - and total is None) # if True, retrieving current revision + latest = all(val is None for val in kwargs.values()) + + revids = kwargs.get('revids') + startid = kwargs.get('startid') + starttime = kwargs.get('starttime') + endid = kwargs.get('endid') + endtime = kwargs.get('endtime') + rvdir = kwargs.get('rvdir') + user = kwargs.get('user') + step = kwargs.get('step')

# check for invalid argument combinations - if (startid is not None or endid is not None) and \ - (starttime is not None or endtime is not None): + if (startid is not None or endid is not None) \ + and (starttime is not None or endtime is not None): raise ValueError( 'loadrevisions: startid/endid combined with starttime/endtime') + if starttime is not None and endtime is not None: if rvdir and starttime >= endtime: raise ValueError( 'loadrevisions: starttime > endtime with rvdir=True') - if (not rvdir) and endtime >= starttime: + + if not rvdir and endtime >= starttime: raise ValueError( 'loadrevisions: endtime > starttime with rvdir=False') + if startid is not None and endid is not None: if rvdir and startid >= endid: raise ValueError( 'loadrevisions: startid > endid with rvdir=True') - if (not rvdir) and endid >= startid: + if not rvdir and endid >= startid: raise ValueError( 'loadrevisions: endid > startid with rvdir=False')

rvargs = {'type_arg': 'info|revisions'} + rvargs['rvprop'] = self._rvprops(content=content)

- rvargs['rvprop'] = ['comment', 'flags', 'ids', 'sha1', 'timestamp', - 'user'] - if self.mw_version >= '1.21': - rvargs['rvprop'].append('contentmodel') - if content: - rvargs['rvprop'].append('content') - if section is not None: - rvargs['rvsection'] = str(section) + if content and section is not None: + rvargs['rvsection'] = str(section) + if revids is None: rvtitle = page.title(with_section=False).encode(self.encoding()) rvargs['titles'] = rvtitle @@ -3822,6 +3831,7 @@ rvargs['rvdir'] = 'newer' elif rvdir is not None: rvargs['rvdir'] = 'older' + if startid: rvargs['rvstartid'] = startid if endid: @@ -3830,13 +3840,16 @@ rvargs['rvstart'] = starttime if endtime: rvargs['rvend'] = endtime + if user: rvargs['rvuser'] = user - elif excludeuser: - rvargs['rvexcludeuser'] = excludeuser + else: + rvargs['rvexcludeuser'] = kwargs.get('excludeuser')

# assemble API request - rvgen = self._generator(api.PropertyGenerator, total=total, **rvargs) + rvgen = self._generator(api.PropertyGenerator, + total=kwargs.get('total'), **rvargs) + if step: rvgen.set_query_increment = step

diff --git a/tests/thanks_tests.py b/tests/thanks_tests.py index e005c82..5c10243 100644 --- a/tests/thanks_tests.py +++ b/tests/thanks_tests.py @@ -1,13 +1,13 @@ # -*- coding: utf-8 -*- """Tests for thanks-related code.""" # -# (C) Pywikibot team, 2016-2019 +# (C) Pywikibot team, 2016-2020 # # Distributed under the terms of the MIT license. # -from __future__ import absolute_import, division, unicode_literals +from contextlib import suppress

-from pywikibot.page import Page, Revision, User +from pywikibot.page import Page, User

from tests.aspects import TestCase from tests import unittest @@ -43,7 +43,7 @@ else: self.skipTest(NO_THANKABLE_REVS) before_time = site.getcurrenttimestamp() - Revision._thank(revid, site, source='pywikibot test') + site.thank_revision(revid, source='pywikibot test') log_entries = site.logevents(logtype='thanks', total=5, page=user, start=before_time, reverse=True) try: @@ -72,8 +72,8 @@ test_page.text += '* ~~~~\n' test_page.save('Pywikibot Thanks test') revid = test_page.latest_revision_id - self.assertAPIError('invalidrecipient', None, Revision._thank, - revid, site, source='pywikibot test') + self.assertAPIError('invalidrecipient', None, site.thank_revision, + revid, source='pywikibot test')

class TestThankRevisionErrors(TestCase): @@ -97,8 +97,8 @@ break else: self.skipTest(NO_THANKABLE_REVS) - self.assertAPIError('invalidrecipient', None, Revision._thank, - revid, site, source='pywikibot test') + self.assertAPIError('invalidrecipient', None, site.thank_revision, + revid, source='pywikibot test')

def test_invalid_revision(self): """Test that passing an invalid revision ID causes an error.""" @@ -106,12 +106,10 @@ invalid_revids = (0, -1, 0.99, 'zero, minus one, and point nine nine', (0, -1, 0.99), [0, -1, 0.99]) for invalid_revid in invalid_revids: - self.assertAPIError('invalidrevision', None, Revision._thank, - invalid_revid, site, source='pywikibot test') + self.assertAPIError('invalidrevision', None, site.thank_revision, + invalid_revid, source='pywikibot test')

if __name__ == '__main__': # pragma: no cover - try: + with suppress(SystemExit): unittest.main() - except SystemExit: - pass