jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/818135 ) Change subject: [bugfix]: fix partial caching in Category.subcategories() ...................................................................... [bugfix]: fix partial caching in Category.subcategories() Partial caching in category.subcategories() leads to incomplete data in subsequent calls. Changes: - save api calls for categories with no subcategories, based on cat.categoryinfo['subcats'] info. At the expense of an extra api call for the base category, for a cleaner design. Subcategories have categoryinfo already cached (no extra api calls needed). - consider cache valid only if all subcategories have been fetched. this is guaranteed only if total=none. - also invalidate cache if called with content=true, after chache is created with content=false in previous calls. Bug: T88217 Change-Id: Ifdd4ae5eb1e78764b8d1aaaa286004bcb7705760 --- M pywikibot/page/_pages.py M tests/category_tests.py 2 files changed, 70 insertions(+), 6 deletions(-) Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/page/_pages.py b/pywikibot/page/_pages.py index 9537cd8..b5ebb4a 100644 --- a/pywikibot/page/_pages.py +++ b/pywikibot/page/_pages.py @@ -2390,15 +2390,23 @@ :param content: if True, retrieve the content of the current version of each category description page (default False) """ + + def is_cache_valid(cache: dict, content: bool) -> bool: + return cache['content'] or not content + + if not self.categoryinfo['subcats']: + return + if not isinstance(recurse, bool) and recurse: recurse = recurse - 1 - if not hasattr(self, '_subcats'): - self._subcats = [] - for member in self.site.categorymembers( + if (not hasattr(self, '_subcats') + or not is_cache_valid(self._subcats, content)): + cache = {'data': [], 'content': content} + + for subcat in self.site.categorymembers( self, member_type='subcat', total=total, content=content): - subcat = Category(member) - self._subcats.append(subcat) + cache['data'].append(subcat) yield subcat if total is not None: total -= 1 @@ -2415,8 +2423,11 @@ total -= 1 if total == 0: return + else: + # cache is valid only if all subcategories are fetched (T88217) + self._subcats = cache else: - for subcat in self._subcats: + for subcat in self._subcats['data']: yield subcat if total is not None: total -= 1 diff --git a/tests/category_tests.py b/tests/category_tests.py index 2e947c2..3bd8f1f 100755 --- a/tests/category_tests.py +++ b/tests/category_tests.py @@ -103,9 +103,62 @@ self.assertIn(c1, subcategories) self.assertNotIn(c2, subcategories) + cat = pywikibot.Category(site, 'Category:Wikipedians by gender') subcategories_total = list(cat.subcategories(total=2)) self.assertLength(subcategories_total, 2) + def test_subcategories_cache_length(self): + """Test the subcategories cache length.""" + site = self.get_site() + + # test cache is valid only if all members of cat are iterated. + cat = pywikibot.Category(site, 'Category:Wikipedians by gender') + subcategories = list(cat.subcategories(total=2)) + self.assertLength(subcategories, 2) + self.assertFalse(hasattr(cat, '_subcats')) + + subcategories = list(cat.subcategories()) + self.assertGreater(len(subcategories), 2) + self.assertTrue(hasattr(cat, '_subcats')) + + # new cat, no cached data. + cat = pywikibot.Category(site, 'Category:Wikipedians by gender') + + # cache not available yet due to partial iteration. + gen = cat.subcategories() + _ = next(gen) + self.assertFalse(hasattr(cat, '_subcats')) + + # cache available. + _ = list(gen) + self.assertTrue(hasattr(cat, '_subcats')) + + def test_subcategories_cache_content(self): + """Test the subcategories cache content.""" + site = self.get_site() + cat = pywikibot.Category(site, 'Category:Wikipedians by gender') + + subcategories = list(cat.subcategories(content=False)) + cache_id_1 = id(cat._subcats) + cache_len_1 = len(subcategories) + subcat = subcategories[0] + self.assertFalse(subcat.has_content()) + + # toggle content. + subcategories = list(cat.subcategories(content=True)) + cache_len_2 = len(subcategories) + cache_id_2 = id(cat._subcats) + subcat = subcategories[0] + self.assertTrue(subcat.has_content()) + # cache reloaded. + self.assertNotEqual(cache_id_1, cache_id_2) + self.assertTrue(cache_len_1, cache_len_2) + + # cache not reloaded. + _ = list(cat.subcategories(content=True)) + cache_id_3 = id(cat._subcats) + self.assertEqual(cache_id_2, cache_id_3) + def test_subcategories_recurse(self): """Test the subcategories method with recurse=True.""" site = self.get_site() -- To view, visit https://gerrit.wikimedia.org/r/c/pywikibot/core/+/818135 To unsubscribe, or for help writing mail filters, visit https://gerrit.wikimedia.org/r/settings Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Change-Id: Ifdd4ae5eb1e78764b8d1aaaa286004bcb7705760 Gerrit-Change-Number: 818135 Gerrit-PatchSet: 6 Gerrit-Owner: Mpaa <mpaa.wiki(a)gmail.com> Gerrit-Reviewer: Xqt <info(a)gno.de> Gerrit-Reviewer: jenkins-bot Gerrit-MessageType: merged