jenkins-bot has submitted this change and it was merged. Change subject: [FIX] API: Don't set 'redirect' value if it couldn't be returned ...................................................................... [FIX] API: Don't set 'redirect' value if it couldn't be returned All values, except 'redirect', were only modified in api.update_page, if they were present. So if they weren't present because they weren't requested, they were left untouched. But the 'redirect' property could signal two states if it wasn't present: False or not requested. It treated it as it was always requested so it stated the redirect status as false, if not 'prop=info' was requested first. This fix only updates 'redirect' if it has been requested which is determined by the value for 'prop' used. If another value is added which is only returned for a specific 'prop' value this can be used there too. For anything else, where it's save to check if it is present and apply changes an empty list does suffice. Bug: 71104 Change-Id: I6224ca5c3de9ba78362681182f9dcc3b384af5ce --- M pywikibot/data/api.py M pywikibot/site.py M tests/page_tests.py 3 files changed, 54 insertions(+), 43 deletions(-) Approvals: John Vandenberg: Looks good to me, approved Mpaa: Looks good to me, but someone else must approve jenkins-bot: Verified diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py index b3ba2d9..fe1d4b7 100644 --- a/pywikibot/data/api.py +++ b/pywikibot/data/api.py @@ -1027,6 +1027,7 @@ if not ('inprop' in kwargs and 'protection' in kwargs['inprop']): appendParams(kwargs, 'inprop', 'protection') appendParams(kwargs, 'iiprop', 'timestamp|user|comment|url|size|sha1|metadata') + self.props = kwargs['prop'].split('|') QueryGenerator.__init__(self, generator=generator, **kwargs) self.resultkey = "pages" # element to look for in result @@ -1038,7 +1039,7 @@ """ p = pywikibot.Page(self.site, pagedata['title'], pagedata['ns']) - update_page(p, pagedata) + update_page(p, pagedata, self.props) return p @@ -1090,7 +1091,13 @@ """ QueryGenerator.__init__(self, prop=prop, **kwargs) + self._props = frozenset(prop.split('|')) self.resultkey = "pages" + + @property + def props(self): + """The requested property names.""" + return self._props class ListGenerator(QueryGenerator): @@ -1195,14 +1202,18 @@ pywikibot.cookie_jar.save() -def update_page(page, pagedict): +def update_page(page, pagedict, props=[]): """Update attributes of Page object page, based on query data in pagedict. @param page: object to be updated @type page: Page @param pagedict: the contents of a "page" element of a query response @type pagedict: dict - + @param prop: the property names which resulted in pagedict. If a missing + value in pagedict can indicate both 'false' and 'not present' the + property which would make the value present must be in the props + parameter. + @type prop: iterable of string """ if "pageid" in pagedict: page._pageid = int(pagedict['pageid']) @@ -1211,7 +1222,8 @@ else: raise AssertionError( "Page %s has neither 'pageid' nor 'missing' attribute" % pagedict['title']) - page._isredir = 'redirect' in pagedict + if 'info' in props: + page._isredir = 'redirect' in pagedict if 'touched' in pagedict: page._timestamp = pagedict['touched'] if 'protection' in pagedict: diff --git a/pywikibot/site.py b/pywikibot/site.py index d4fe555..f0eefe0 100644 --- a/pywikibot/site.py +++ b/pywikibot/site.py @@ -2111,6 +2111,16 @@ except api.APIError: # May occur if you are not logged in (no API read permissions). return (0, 0, 0) + def _update_page(self, page, query, method_name): + for pageitem in query: + if not self.sametitle(pageitem['title'], + page.title(withSection=False)): + pywikibot.warning( + u"{0}: Query on {1} returned data on '{2}'".format( + method_name, page, pageitem['title'])) + continue + api.update_page(page, pageitem, query.props) + def loadpageinfo(self, page, preload=False): """Load page info from api and store in page attributes.""" title = page.title(withSection=False) @@ -2122,14 +2132,7 @@ type_arg="info", titles=title.encode(self.encoding()), inprop=inprop) - - for pageitem in query: - if not self.sametitle(pageitem['title'], title): - pywikibot.warning( - u"loadpageinfo: Query on %s returned data on '%s'" - % (page, pageitem['title'])) - continue - api.update_page(page, pageitem) + self._update_page(page, query, 'loadpageinfo') def loadcoordinfo(self, page): """Load [[mw:Extension:GeoData]] info.""" @@ -2141,13 +2144,7 @@ 'country', 'region', 'globe'], coprimary='all') - for pageitem in query: - if not self.sametitle(pageitem['title'], title): - pywikibot.warning( - u"loadcoordinfo: Query on %s returned data on '%s'" - % (page, pageitem['title'])) - continue - api.update_page(page, pageitem) + self._update_page(page, query, 'loadcoordinfo') def loadpageprops(self, page): title = page.title(withSection=False) @@ -2155,13 +2152,7 @@ type_arg="pageprops", titles=title.encode(self.encoding()), ) - for pageitem in query: - if not self.sametitle(pageitem['title'], title): - pywikibot.warning( - u"loadpageprops: Query on %s returned data on '%s'" - % (page, pageitem['title'])) - continue - api.update_page(page, pageitem) + self._update_page(page, query, 'loadpageprops') def loadimageinfo(self, page, history=False): """Load image info from api and save in page attributes. @@ -2184,7 +2175,7 @@ raise Error( u"loadimageinfo: Query on %s returned data on '%s'" % (page, pageitem['title'])) - api.update_page(page, pageitem) + api.update_page(page, pageitem, query.props) if "imageinfo" not in pageitem: if "missing" in pageitem: raise NoPage(page) @@ -2206,13 +2197,7 @@ type_arg="flowinfo", titles=title.encode(self.encoding()), ) - for pageitem in query: - if not self.sametitle(pageitem['title'], title): - pywikibot.warning( - u"loadflowinfo: Query on %s returned data on '%s'" - % (page, pageitem['title'])) - continue - api.update_page(page, pageitem) + self._update_page(page, query, 'loadflowinfo') def page_exists(self, page): """Return True if and only if page is an existing page on site.""" @@ -2292,7 +2277,7 @@ # there should be only one value in 'pages', and it is the target if self.sametitle(pagedata['title'], target_title): target = pywikibot.Page(self, pagedata['title'], pagedata['ns']) - api.update_page(target, pagedata) + api.update_page(target, pagedata, ['info']) page._redirtarget = target else: # double redirect; target is an intermediate redirect @@ -2360,7 +2345,7 @@ pywikibot.debug(u"titles=%s" % list(cache.keys()), _logger) continue page = cache[pagedata['title']] - api.update_page(page, pagedata) + api.update_page(page, pagedata, rvgen.props) yield page def validate_tokens(self, types): @@ -2859,7 +2844,7 @@ % (page, pagedata['title'])) if "missing" in pagedata: raise NoPage(page) - api.update_page(page, pagedata) + api.update_page(page, pagedata, rvgen.props) def pageinterwiki(self, page): # No such function in the API (this method isn't called anywhere) @@ -2916,12 +2901,7 @@ ciquery = self._generator(api.PropertyGenerator, type_arg="categoryinfo", titles=cititle.encode(self.encoding())) - for pageitem in ciquery: - if not self.sametitle(pageitem['title'], cititle): - raise Error( - u"categoryinfo: Query on %s returned data on '%s'" - % (category, pageitem['title'])) - api.update_page(category, pageitem) + self._update_page(category, ciquery, 'categoryinfo') def categoryinfo(self, category): if not hasattr(category, "_catinfo"): diff --git a/tests/page_tests.py b/tests/page_tests.py index 5883922..eed2993 100644 --- a/tests/page_tests.py +++ b/tests/page_tests.py @@ -452,6 +452,25 @@ s = repr(page) self.assertIsInstance(s, str) + def test_redirect(self): + """Test that the redirect option is set correctly.""" + mysite = self.get_site() + for page in mysite.allpages(filterredir=True, total=1): + break + else: + unittest.SkipTest('No redirect pages on site {0!r}'.format(mysite)) + # This page is already initialised + self.assertTrue(hasattr(page, '_isredir')) + # call api.update_page without prop=info + del page._isredir + page.isDisambig() + self.assertTrue(page.isRedirectPage()) + + page_copy = pywikibot.Page(mysite, page.title()) + self.assertFalse(hasattr(page_copy, '_isredir')) + page_copy.isDisambig() + self.assertTrue(page_copy.isRedirectPage()) + # methods that still need tests implemented or expanded: # def autoFormat(self): -- To view, visit https://gerrit.wikimedia.org/r/162157 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6224ca5c3de9ba78362681182f9dcc3b384af5ce Gerrit-PatchSet: 2 Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Owner: XZise &lt;CommodoreFabianus(a)gmx.de&gt; Gerrit-Reviewer: John Vandenberg &lt;jayvdb(a)gmail.com&gt; Gerrit-Reviewer: Ladsgroup &lt;ladsgroup(a)gmail.com&gt; Gerrit-Reviewer: Merlijn van Deen &lt;valhallasw(a)arctus.nl&gt; Gerrit-Reviewer: Mpaa &lt;mpaa.wiki(a)gmail.com&gt; Gerrit-Reviewer: XZise &lt;CommodoreFabianus(a)gmx.de&gt; Gerrit-Reviewer: jenkins-bot <>