jenkins-bot has submitted this change and it was merged.

Change subject: (perf) Parse ItemPage title once ......................................................................

(perf) Parse ItemPage title once

ItemPage may be constructed without a page title, as it is delay loaded from the wikibase_item of a page on another site.

Optimise ItemPage.title() so it does not reparse the Link title every invocation.

Add test case for instantiating an empty ItemPage and setting .id to a valid Qxx, and another case for altering .id highlighting a bug yet to be fixed.

Split from: If7ca06adbb4b9932bba0abffc7588afcb320e934

Change-Id: I95a053a1ea17611498fcb5601c09a83d82f91fe8 --- M pywikibot/page.py M tests/wikibase_tests.py 2 files changed, 49 insertions(+), 3 deletions(-)

Approvals: John Vandenberg: Looks good to me, but someone else must approve Legoktm: Looks good to me, approved jenkins-bot: Verified

diff --git a/pywikibot/page.py b/pywikibot/page.py index 4675a72..7a0d1bd 100644 --- a/pywikibot/page.py +++ b/pywikibot/page.py @@ -2580,10 +2580,16 @@ self._isredir = False # Wikibase pages cannot be a redirect

def title(self, **kwargs): - """ Page title. """ + """ Page title. + + If the item was instantiated without an ID, + fetch the ID and reparse the title. + """ if self.namespace() == 0: - self._link._text = self.getID() - del self._link._title + self.getID() + if self._link._text != self.id: + self._link._text = self.id + del self._link._title return Page(self).title(**kwargs)

@deprecated("_defined_by") diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py index 4059c4a..aca60b3 100644 --- a/tests/wikibase_tests.py +++ b/tests/wikibase_tests.py @@ -167,6 +167,46 @@ item.get() self.assertEquals(hasattr(item, '_content'), True)

+ def test_load_item_set_id(self): + """Test setting item.id attribute on empty item.""" + item = pywikibot.ItemPage(wikidata, '-1') + self.assertEquals(item._link._title, '-1') + item.id = 'Q60' + self.assertEquals(hasattr(item, '_content'), False) + self.assertEquals(item.getID(), 'Q60') + self.assertEquals(hasattr(item, '_content'), False) + item.get() + self.assertEquals(hasattr(item, '_content'), True) + self.assertTrue('en' in item.labels) + self.assertEquals(item.labels['en'], 'New York City') + self.assertEquals(item.title(), 'Q60') + + def test_reuse_item_set_id(self): + """ + Test modifying item.id attribute. + + Some scripts are using item.id = 'Q60' semantics, which does work + but modifying item.id does not currently work, and this test + highlights that it breaks silently. + """ + item = pywikibot.ItemPage(wikidata, 'Q60') + item.get() + self.assertEquals(item.labels['en'], 'New York City') + + # When the id attribute is modified, the ItemPage goes into + # an inconsistent state. + item.id = 'Q5296' + # The title is updated correctly + self.assertEquals(item.title(), 'Q5296') + + # This del has no effect on the test; it is here to demonstrate that + # it doesnt help to clear this piece of saved state. + del item._content + # The labels are not updated; assertion showing undesirable behaviour: + self.assertEquals(item.labels['en'], 'New York City') + # TODO: This is the assertion that this test should be using: + #self.assertTrue(item.labels['en'].lower().endswith('main page')) + def test_empty_item(self): # should not raise an error as the constructor only requires # the site parameter, with the title parameter defaulted to None