jenkins-bot submitted this change.

View Change

Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified
[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
but ignore DryRequest rejecting Exception
- 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

Bug: T267770
Change-Id: I03f23af85371d4246f41db70c7148a27d9b34c35
---
M pywikibot/page/__init__.py
M tests/page_tests.py
2 files changed, 100 insertions(+), 134 deletions(-)

diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py
index 96cee52..8e6fe80 100644
--- a/pywikibot/page/__init__.py
+++ b/pywikibot/page/__init__.py
@@ -620,26 +620,26 @@
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.botMayEdit()
- return self.latest_revision.text
-
@text.setter
- def text(self, value: str):
- """
- Update the current (edited) wikitext.
+ def text(self, value: Optional[str]):
+ """Update the current (edited) wikitext.

@param value: New value or None
"""
+ try:
+ self.botMayEdit() # T262136, T267770
+ except Exception as e:
+ # dry tests aren't able to make an API call but are
+ # but are rejected by an Exception; ignore it then.
+ if not str(e).startswith('DryRequest rejecting request:'):
+ raise
+
del self.text
self._text = None if value is None else str(value)

diff --git a/tests/page_tests.py b/tests/page_tests.py
index 9fd5f06..1bc7743 100644
--- a/tests/page_tests.py
+++ b/tests/page_tests.py
@@ -739,133 +739,108 @@
'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()
+
+ def _run_test(self, template, user, expected_result):
+ """Run a single template test."""
+ del self.page.text
+ self.page._text = template % {'user': user}
+ with self.subTest(template=template, user=user):
+ self.assertEqual(self.page.botMayEdit(), expected_result)
+ self.page.botMayEdit.cache_clear()
+
@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:
+ self._run_test(template, user, True)

@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:
+ self._run_test(template, user, False)
+
+ @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:
+ self._run_test(template, user, True)

# 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:
+ self._run_test(template, user, False)

@mock.patch.object(config, 'ignore_bot_templates', False)
def test_bot_may_edit_inuse(self):
@@ -873,7 +848,7 @@
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):
@@ -891,19 +866,10 @@

def test_bot_may_edit_page_set_text(self):
"""Test botMayEdit for existing page when assigning text first."""
- content = 'Does page may be changed if {{nobots}} template is found?'
- # test the page with assigning text first
- with self.subTest(content=content):
- page = pywikibot.Page(self.site, 'Pywikibot nobots test')
- page.text = content
- self.assertFalse(page.botMayEdit())
-
- @unittest.expectedFailure
- def test_bot_may_edit_page_set_text_failing(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:

To view, visit change 645925. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: I03f23af85371d4246f41db70c7148a27d9b34c35
Gerrit-Change-Number: 645925
Gerrit-PatchSet: 4
Gerrit-Owner: Xqt <info@gno.de>
Gerrit-Reviewer: Mpaa <mpaa.wiki@gmail.com>
Gerrit-Reviewer: Xqt <info@gno.de>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged