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

Change subject: [bugfix] Change meaning of BasePage.text ......................................................................

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

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

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