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 <[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