jenkins-bot submitted this change.

View Change

[bugfix] Change meaning of BasePage.text

- BasePage._text now only caches content saved using
  the setter of BasePage.text. Caching two distinct
  things in a single property was source of the
  confusion behind the problem. Since Revisions are
  immutable (or should be), the live content is cached
  in the latest revision object.
- BasePage.text returns BasePage._text if it is set,
  otherwise returns the live content.
- If BasePage._text has been set and the caller wants
  to work with the live content, they should use
  BasePage.get() or set BasePage.text to None first.

Since _text doesn't cache live content anymore, do not
delete it when page information is loaded.

Bug: T260472
Change-Id: I8c8fbcd57998ea3d68e047cf6e5a0a1241156ceb
---
M pywikibot/data/api.py
M pywikibot/page/__init__.py
M tests/basepage_tests.py
3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py
index 122f5fd..fd72de1 100644
--- a/pywikibot/data/api.py
+++ b/pywikibot/data/api.py
@@ -3394,7 +3394,6 @@
 
     if 'lastrevid' in pagedict:
         page.latest_revision_id = pagedict['lastrevid']
-        del page.text
 
     if 'imageinfo' in pagedict:
         assert(isinstance(page, pywikibot.FilePage))
diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py
index a286f29..2610429 100644
--- a/pywikibot/page/__init__.py
+++ b/pywikibot/page/__init__.py
@@ -609,13 +609,13 @@
         @return: text of the page
         @rtype: str
         """
-        if not hasattr(self, '_text') or self._text is None:
-            try:
-                self._text = self.get(get_redirect=True)
-            except pywikibot.NoPage:
-                # TODO: what other exceptions might be returned?
-                self._text = ''
-        return self._text
+        if getattr(self, '_text', None) is not None:
+            return self._text
+        try:
+            return self.get(get_redirect=True)
+        except pywikibot.NoPage:
+            # TODO: what other exceptions might be returned?
+            return ''
 
     @text.setter
     def text(self, value):
@@ -625,9 +625,8 @@
         @param value: New value or None
         @type value: basestring
         """
+        del self.text
         self._text = None if value is None else str(value)
-        if hasattr(self, '_raw_extracted_templates'):
-            del self._raw_extracted_templates
 
     @text.deleter
     def text(self):
@@ -1452,6 +1451,8 @@
 
         minor and botflag parameters are set to False which prevents hiding
         the edit when it becomes a real edit due to a bug.
+
+        @note: This discards content saved to self.text.
         """
         if self.exists():
             # ensure always get the page text and not to change it.
diff --git a/tests/basepage_tests.py b/tests/basepage_tests.py
index 8964049..98c1443 100644
--- a/tests/basepage_tests.py
+++ b/tests/basepage_tests.py
@@ -53,6 +53,11 @@
         self.assertTrue(hasattr(page, '_revisions'))
         self.assertFalse(page._revisions)
 
+        # verify that initializing the page content
+        # does not discard the custom text
+        custom_text = 'foobar'
+        page.text = custom_text
+
         self.site.loadrevisions(page, total=1)
 
         self.assertTrue(hasattr(page, '_revid'))
@@ -60,25 +65,42 @@
         self.assertLength(page._revisions, 1)
         self.assertIn(page._revid, page._revisions)
 
+        self.assertEqual(page._text, custom_text)
+        self.assertEqual(page.text, page._text)
+        del page.text
+
         self.assertFalse(hasattr(page, '_text'))
         self.assertIsNone(page._revisions[page._revid].text)
         self.assertFalse(hasattr(page, '_text'))
         self.assertIsNone(page._latest_cached_revision())
 
+        page.text = custom_text
+
         self.site.loadrevisions(page, total=1, content=True)
-        self.assertFalse(hasattr(page, '_text'))
+
         self.assertIsNotNone(page._latest_cached_revision())
+        self.assertEqual(page._text, custom_text)
+        self.assertEqual(page.text, page._text)
+        del page.text
+        self.assertFalse(hasattr(page, '_text'))
 
         # Verify that calling .text doesn't call loadrevisions again
         loadrevisions = self.site.loadrevisions
         try:
             self.site.loadrevisions = None
-            self.assertIsNotNone(page.text)
+            loaded_text = page.text
+            self.assertIsNotNone(loaded_text)
+            self.assertFalse(hasattr(page, '_text'))
+            page.text = custom_text
+            self.assertEqual(page.get(), loaded_text)
+            self.assertEqual(page._text, custom_text)
+            self.assertEqual(page.text, page._text)
+            del page.text
+            self.assertFalse(hasattr(page, '_text'))
+            self.assertEqual(page.text, loaded_text)
         finally:
             self.site.loadrevisions = loadrevisions
 
-        self.assertTrue(hasattr(page, '_text'))
-
 
 class BasePageMethodsTestBase(BasePageTestBase):
 

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