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

Change subject: Report site&title for lazy loaded ItemPage ......................................................................

Report site&title for lazy loaded ItemPage

When lazy loading the ID of a ItemPage, NoPage exception is thrown if the item is 'missing'. Currently the Wikibase API does not distinguish between a) title not existing on a site, and b) site&title not having a wikibase item associated with it. Both are 'missing'. Bug 68251.

If the site&title does not exist, the NoPage exception should report the site&title which does not exist, so this appears in the logging rather than reporting that 'Item -1' does not exist.

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

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

diff --git a/pywikibot/page.py b/pywikibot/page.py index 3396ed1..6d45fba 100644 --- a/pywikibot/page.py +++ b/pywikibot/page.py @@ -2645,6 +2645,7 @@ @type force: bool @param args: may be used to specify custom props. """ + lazy_loading_id = not hasattr(self, 'id') and hasattr(self, '_site') if force or not hasattr(self, '_content'): data = self.repo.loadcontent(self._defined_by(), *args) self.id = list(data.keys())[0] @@ -2652,7 +2653,12 @@ if 'lastrevid' in self._content: self.lastrevid = self._content['lastrevid'] else: + if lazy_loading_id: + p = Page(self._site, self._title) + if not p.exists(): + raise pywikibot.NoPage(p) raise pywikibot.NoPage(self) + # aliases self.aliases = {} if 'aliases' in self._content: diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py index ba91af6..b4bbb96 100644 --- a/tests/wikibase_tests.py +++ b/tests/wikibase_tests.py @@ -224,6 +224,15 @@ self.assertRaises(pywikibot.InvalidTitle, pywikibot.ItemPage.fromPage, page)

def _test_fromPage_noitem(self, link): + """Helper function to test a page without an associated item. + + It tests two of the ways to fetch an item: + 1. the Page already has props, which should contain a item id if + present, and that item id is used to instantiate the item, and + 2. the page doesnt have props, in which case the site&titles is + used to lookup the item id, but that lookup occurs after + instantiation, during the first attempt to use the data item. + """ for props in [True, False]: for method in ['title', 'get', 'getID', 'exists']: page = pywikibot.Page(link) @@ -243,8 +252,15 @@ else: self.assertRaises(pywikibot.NoPage, getattr(item, method))

- # invoking any of those methods changes the title to '-1' - self.assertEquals(item._link._title, '-1') + # The invocation above of a fetching method shouldnt change + # the local item, but it does! The title changes to '-1'. + # + # However when identifying the item for 'en:Test page' + # (a deleted page), the exception handling is smarter, and no + # local data is modified in this scenario. This case is + # separately tested in test_fromPage_missing_lazy. + if link.title != 'Test page': + self.assertEquals(item._link._title, '-1')

self.assertEquals(hasattr(item, '_content'), True)

@@ -266,6 +282,21 @@ link = page._link self._test_fromPage_noitem(link)

+ def test_fromPage_missing_lazy(self): + """Test lazy loading of item from nonexistent source page.""" + # this is a deleted page, and should not have a wikidata item + link = pywikibot.page.Link("Test page", site) + page = pywikibot.Page(link) + item = pywikibot.ItemPage.fromPage(page) + + # Now verify that delay loading will result in the desired semantics. + # It should not raise NoPage on the wikibase item which has a title + # like '-1' or 'Null', as that is useless to determine the cause + # without a full debug log. + # It should raise NoPage on the page, as that is what the + # bot operator needs to see in the log output. + self.assertRaisesRegexp(pywikibot.NoPage, 'Test page', item.get) +

class TestPropertyPage(PywikibotTestCase):