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