Xqt has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/645363 )

Change subject: Revert "[bugfix] Check for {{bots}}/{{nobots}} in Page.text setter" ......................................................................

Revert "[bugfix] Check for {{bots}}/{{nobots}} in Page.text setter"

This reverts commit bcc30f5a6fa95f4558ae43721bb5703a80cd0c93.

Reason for revert: Seems some tests are magically using DryPage due to unknown reason with CI tests. But it works locally. Strange.

Bug: T267770 Change-Id: I6f4157903ffa31b54db6f3af18c093df1b1c9234 --- M pywikibot/page/__init__.py M tests/aspects.py M tests/page_tests.py M tests/utils.py 4 files changed, 130 insertions(+), 132 deletions(-)

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

diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py index d0c9006..fdc1973 100644 --- a/pywikibot/page/__init__.py +++ b/pywikibot/page/__init__.py @@ -62,13 +62,10 @@ 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?://' @@ -619,12 +616,19 @@ if getattr(self, '_text', None) is not None: return self._text

+ if hasattr(self, '_revid'): + return self.latest_revision.text + try: - return self.get(get_redirect=True) + 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): """ @@ -632,8 +636,6 @@

@param value: New value or None """ - self.botMayEdit() # T262136, T267770 - del self.text self._text = None if value is None else str(value)

@@ -1090,7 +1092,6 @@ """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. @@ -1106,6 +1107,9 @@ 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 61d70a8..8a8d29b 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, DryPage, DryRequest, + execute_pwb, DrySite, DryRequest, WarningSourceSkipContextManager, AssertAPIErrorContextManager, )

@@ -385,7 +385,6 @@ 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 7962a15..e4ef897 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), "DryPage('Ō')") - self.assertEqual('%r' % self.page, "DryPage('Ō')") - self.assertEqual('{0!r}'.format(self.page), "DryPage('Ō')") + self.assertEqual(repr(self.page), "Page('Ō')") + self.assertEqual('%r' % self.page, "Page('Ō')") + self.assertEqual('{0!r}'.format(self.page), "Page('Ō')")

class TestPageReprASCII(TestPageBaseUnicode): @@ -739,116 +739,133 @@ '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_ok(self): + def test_bot_may_edit_nobots(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() - 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() + + # 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())

@mock.patch.object(config, 'ignore_bot_templates', False) - 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): + def test_bot_may_edit_bots(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() - 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() + + # 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())

# Ignore empty keyword parameter with bots for param in ('allow', 'deny', 'empty_parameter'): - del self.page.text - self.page._text = '{{bots|%s=}}' % param + 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()

- @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() + # 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_inuse(self): @@ -856,37 +873,19 @@ 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_missing_page(self): - """Test botMayEdit for not existent page.""" + def test_bot_may_edit_page(self): + """Test botMayEdit when changing content.""" 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 67fa874..3c12e3b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -435,10 +435,6 @@ """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):