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

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
  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(-)

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



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 https://gerrit.wikimedia.org/r/c/pywikibot/core/+/645925
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: I03f23af85371d4246f41db70c7148a27d9b34c35
Gerrit-Change-Number: 645925
Gerrit-PatchSet: 4
Gerrit-Owner: Xqt <[email protected]>
Gerrit-Reviewer: Mpaa <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged
_______________________________________________
Pywikibot-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/pywikibot-commits

Reply via email to