jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/640800 ) Change subject: [bugfix] Check for {{bots}}/{{nobots}} in Page.text setter ...................................................................... [bugfix] Check for {{bots}}/{{nobots}} in Page.text setter This reverts fdfae7bb446a6 and moves early check into BasePage.text setter - revert fdfae7bb446a6 change for BasePage - check for botMayEdit in text setter first - cache botMayEdit result - added a test which assigns a text first without reading from api - update old tests with those steps: - call text deleter to remove "_raw_extracted_templates" cache - add template to "_text" attribute directly to bypass the botMayEdit check - make the test - clear the botMayEdit cache - patch unittest for "dry" option using a DryPage which prevents api call with botMayEdit method. Bug: T267770 Change-Id: I986d3e2b3828266442cc9127dd792196d585b532 --- M pywikibot/page/__init__.py M tests/aspects.py M tests/page_tests.py M tests/utils.py 4 files changed, 132 insertions(+), 130 deletions(-) Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py index fdc1973..d0c9006 100644 --- a/pywikibot/page/__init__.py +++ b/pywikibot/page/__init__.py @@ -62,10 +62,13 @@ from pywikibot.tools import is_IP if PYTHON_VERSION >= (3, 9): + from functools import cache Dict = dict List = list else: + from functools import lru_cache from typing import Dict, List + cache = lru_cache(None) PROTOCOL_REGEX = r'\Ahttps?://' @@ -616,19 +619,12 @@ if getattr(self, '_text', None) is not None: return self._text - if hasattr(self, '_revid'): - return self.latest_revision.text - try: - self.get(get_redirect=True) + return self.get(get_redirect=True) except pywikibot.NoPage: # TODO: what other exceptions might be returned? return '' - # check botMayEdit on a very early state (T262136) - self._bot_may_edit = self.botMayEdit() - return self.latest_revision.text - @text.setter def text(self, value: str): """ @@ -636,6 +632,8 @@ @param value: New value or None """ + self.botMayEdit() # T262136, T267770 + del self.text self._text = None if value is None else str(value) @@ -1092,6 +1090,7 @@ """DEPRECATED. Determine whether the page may be edited.""" return self.has_permission() + @cache def botMayEdit(self) -> bool: """ Determine whether the active bot is allowed to edit the page. @@ -1107,9 +1106,6 @@ to override this by setting ignore_bot_templates=True in user-config.py, or using page.put(force=True). """ - if hasattr(self, '_bot_may_edit'): - return self._bot_may_edit - if not hasattr(self, 'templatesWithParams'): return True diff --git a/tests/aspects.py b/tests/aspects.py index 8a8d29b..61d70a8 100644 --- a/tests/aspects.py +++ b/tests/aspects.py @@ -39,7 +39,7 @@ from tests import ( safe_repr, unittest, patch_request, unpatch_request, unittest_print) from tests.utils import ( - execute_pwb, DrySite, DryRequest, + execute_pwb, DrySite, DryPage, DryRequest, WarningSourceSkipContextManager, AssertAPIErrorContextManager, ) @@ -385,6 +385,7 @@ config.site_interface = SiteNotPermitted pywikibot.data.api.Request = DryRequest + pywikibot.Page = DryPage # T267770 self.old_convert = pywikibot.Claim.TARGET_CONVERTER['commonsMedia'] pywikibot.Claim.TARGET_CONVERTER['commonsMedia'] = ( lambda value, site: pywikibot.FilePage( diff --git a/tests/page_tests.py b/tests/page_tests.py index e4ef897..7962a15 100644 --- a/tests/page_tests.py +++ b/tests/page_tests.py @@ -698,9 +698,9 @@ def test_unicode_value(self): """Test to capture actual Python result pre unicode_literals.""" - self.assertEqual(repr(self.page), "Page('Ō')") - self.assertEqual('%r' % self.page, "Page('Ō')") - self.assertEqual('{0!r}'.format(self.page), "Page('Ō')") + self.assertEqual(repr(self.page), "DryPage('Ō')") + self.assertEqual('%r' % self.page, "DryPage('Ō')") + self.assertEqual('{0!r}'.format(self.page), "DryPage('Ō')") class TestPageReprASCII(TestPageBaseUnicode): @@ -739,133 +739,116 @@ 'Page {} exists! Change page name in tests/page_tests.py' .format(self.page.title())) + def tearDown(self): + """Cleanup cache.""" + self.page.botMayEdit.cache_clear() + super().tearDown() + @mock.patch.object(config, 'ignore_bot_templates', False) - def test_bot_may_edit_nobots(self): + def test_bot_may_edit_nobots_ok(self): """Test with {{nobots}} that bot is allowed to edit.""" + templates = ( + # Ban all compliant bots not in the list, syntax for de wp. + '{{nobots|HagermanBot,Werdnabot}}', + # Ignore second parameter + '{{nobots|MyBot|%(user)s}}', + ) + self.page._templates = [pywikibot.Page(self.site, 'Template:Nobots')] user = self.site.user() - - # Ban all compliant bots (shortcut). - self.page.text = '{{nobots}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ban all compliant bots not in the list, syntax for de wp. - self.page.text = '{{nobots|HagermanBot,Werdnabot}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ban all compliant bots not in the list, syntax for de wp. - self.page.text = '{{nobots|%s, HagermanBot,Werdnabot}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ban all bots, syntax for de wp. - self.page.text = '{{nobots|all}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Decline wrong nobots parameter - self.page.text = '{{nobots|allow=%s}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Decline wrong nobots parameter - self.page.text = '{{nobots|deny=%s}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Decline wrong nobots parameter - self.page.text = '{{nobots|decline=%s}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Decline empty keyword parameter with nobots - self.page.text = '{{nobots|with_empty_parameter=}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ignore second parameter - self.page.text = '{{nobots|%s|MyBot}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ignore second parameter - self.page.text = '{{nobots|MyBot|%s}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) + for template in templates: + del self.page.text + self.page._text = template % {'user': user} + with self.subTest(template=template, user=user): + self.assertTrue(self.page.botMayEdit()) + self.page.botMayEdit.cache_clear() @mock.patch.object(config, 'ignore_bot_templates', False) - def test_bot_may_edit_bots(self): + def test_bot_may_edit_nobots_nok(self): + """Test with {{nobots}} that bot is not allowed to edit.""" + templates = ( + # Ban all compliant bots (shortcut) + '{{nobots}}', + # Ban all compliant bots not in the list, syntax for de wp + '{{nobots|%(user)s, HagermanBot,Werdnabot}}', + # Ban all bots, syntax for de wp + '{{nobots|all}}', + # Decline wrong nobots parameter + '{{nobots|allow=%(user)s}}', + '{{nobots|deny=%(user)s}}', + '{{nobots|decline=%(user)s}}', + # Decline empty keyword parameter with nobots + '{{nobots|with_empty_parameter=}}', + # Ignore second parameter + '{{nobots|%(user)s|MyBot}}', + ) + + self.page._templates = [pywikibot.Page(self.site, 'Template:Nobots')] + user = self.site.user() + for template in templates: + del self.page.text + self.page._text = template % {'user': user} + with self.subTest(template=template, user=user): + self.assertFalse(self.page.botMayEdit()) + self.page.botMayEdit.cache_clear() + + @mock.patch.object(config, 'ignore_bot_templates', False) + def test_bot_may_edit_bots_ok(self): """Test with {{bots}} that bot is allowed to edit.""" + templates = ( + '{{bots}}', # Allow all bots (shortcut) + # Ban all compliant bots in the list + '{{bots|deny=HagermanBot,Werdnabot}}', + # Ban all compliant bots not in the list + '{{bots|allow=%(user)s, HagermanBot}}', + # Allow all bots + '{{bots|allow=all}}', + '{{bots|deny=none}}', + # Ignore missing named parameter + '{{bots|%(user)s}}', # Ignore missing named parameter + # Ignore unknown keyword parameter with bots + '{{bots|with=unknown_parameter}}', + # Ignore unknown empty parameter keyword with bots + '{{bots|with_empty_parameter=}}', + ) + self.page._templates = [pywikibot.Page(self.site, 'Template:Bots')] user = self.site.user() - - # Allow all bots (shortcut). - self.page.text = '{{bots}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ban all compliant bots not in the list. - self.page.text = '{{bots|allow=HagermanBot,Werdnabot}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ban all compliant bots in the list. - self.page.text = '{{bots|deny=HagermanBot,Werdnabot}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ban all compliant bots not in the list. - self.page.text = '{{bots|allow=%s, HagermanBot}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ban all compliant bots in the list. - self.page.text = '{{bots|deny=%s, HagermanBot}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Allow all bots. - self.page.text = '{{bots|allow=all}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ban all compliant bots. - self.page.text = '{{bots|allow=none}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Ban all compliant bots. - self.page.text = '{{bots|deny=all}}' - with self.subTest(template=self.page.text, user=user): - self.assertFalse(self.page.botMayEdit()) - - # Allow all bots. - self.page.text = '{{bots|deny=none}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ignore missing named parameter. - self.page.text = '{{bots|%s}}' % user - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) + for template in templates: + del self.page.text + self.page._text = template % {'user': user} + with self.subTest(template=template, user=user): + self.assertTrue(self.page.botMayEdit()) + self.page.botMayEdit.cache_clear() # Ignore empty keyword parameter with bots for param in ('allow', 'deny', 'empty_parameter'): - self.page.text = '{{bots|%s=}}' % param + del self.page.text + self.page._text = '{{bots|%s=}}' % param with self.subTest(template=self.page.text, user=user, param=param): self.assertTrue(self.page.botMayEdit()) + self.page.botMayEdit.cache_clear() - # Ignore unknown keyword parameter with bots - self.page.text = '{{bots|with=unknown_parameter}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) - - # Ignore unknown empty parameter keyword with bots - self.page.text = '{{bots|with_empty_parameter=}}' - with self.subTest(template=self.page.text, user=user): - self.assertTrue(self.page.botMayEdit()) + @mock.patch.object(config, 'ignore_bot_templates', False) + def test_bot_may_edit_bots_nok(self): + """Test with {{bots}} that bot is not allowed to edit.""" + templates = ( + # Ban all compliant bots not in the list + '{{bots|allow=HagermanBot,Werdnabot}}', + # Ban all compliant bots in the list + '{{bots|deny=%(user)s, HagermanBot}}', + # Ban all compliant bots + '{{bots|allow=none}}', + '{{bots|deny=all}}', + ) + self.page._templates = [pywikibot.Page(self.site, 'Template:Bots')] + user = self.site.user() + for template in templates: + del self.page.text + self.page._text = template % {'user': user} + with self.subTest(template=template, user=user): + self.assertFalse(self.page.botMayEdit()) + self.page.botMayEdit.cache_clear() @mock.patch.object(config, 'ignore_bot_templates', False) def test_bot_may_edit_inuse(self): @@ -873,19 +856,37 @@ self.page._templates = [pywikibot.Page(self.site, 'Template:In use')] # Ban all users including bots. - self.page.text = '{{in use}}' + self.page._text = '{{in use}}' self.assertFalse(self.page.botMayEdit()) - def test_bot_may_edit_page(self): - """Test botMayEdit when changing content.""" + def test_bot_may_edit_missing_page(self): + """Test botMayEdit for not existent page.""" self.assertTrue(self.page.botMayEdit()) self.page.text = '{{nobots}}' self.assertTrue(self.page.botMayEdit()) + + def test_bot_may_edit_page_nobots(self): + """Test botMayEdit for existing page with nobots template.""" page = pywikibot.Page(self.site, 'Pywikibot nobots test') self.assertFalse(page.botMayEdit()) page.text = '' self.assertFalse(page.botMayEdit()) + def test_bot_may_edit_page_set_text(self): + """Test botMayEdit for existing page when assigning text first.""" + contents = ( + 'Does page may be changed if content is not read first?', + 'Does page may be changed if {{bots}} template is found?', + 'Does page may be changed if {{nobots}} template is found?' + ) + # test the page with assigning text first + for content in contents: + with self.subTest(content=content): + page = pywikibot.Page(self.site, 'Pywikibot nobots test') + page.text = content + self.assertFalse(page.botMayEdit()) + del page + class TestPageHistory(DefaultSiteTestCase): diff --git a/tests/utils.py b/tests/utils.py index 3c12e3b..67fa874 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -435,6 +435,10 @@ """Return disambig status stored in _disambig.""" return self._disambig + def botMayEdit(self): + """Ignore botMayEdit life content call with text setter (T267770).""" + return None + class FakeLoginManager(pywikibot.data.api.LoginManager): -- To view, visit https://gerrit.wikimedia.org/r/c/pywikibot/core/+/640800 To unsubscribe, or for help writing mail filters, visit https://gerrit.wikimedia.org/r/settings Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Change-Id: I986d3e2b3828266442cc9127dd792196d585b532 Gerrit-Change-Number: 640800 Gerrit-PatchSet: 5 Gerrit-Owner: Xqt <info(a)gno.de> Gerrit-Reviewer: Mpaa <mpaa.wiki(a)gmail.com> Gerrit-Reviewer: Xqt <info(a)gno.de> Gerrit-Reviewer: jenkins-bot Gerrit-MessageType: merged