jenkins-bot merged this change.

View Change

[bugfix] always use Site.userinfo in favour of Site._userinfo

- always use Site.userinfo in favour of Site._userinfo to ensure
  _siteinfo attribute is given
- print a warning if no user is logged in when retrieving userinfo
- check for "anon" and "id" in userinfo for logged_in() method
- additional tests in TestDrySite.test_logged_in()
- use subTest for TestDrySite.test_logged_in()

Bug: T243794
Change-Id: I9c1072d2af2c7c237a0f89c8a0482b644d2e92f0
---
M pywikibot/site.py
M tests/dry_site_tests.py
2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/pywikibot/site.py b/pywikibot/site.py
index 02edc0c..040c8d8 100644
--- a/pywikibot/site.py
+++ b/pywikibot/site.py
@@ -1959,15 +1959,17 @@
     def logged_in(self):
         """Verify the bot is logged into the site as the expected user.
 
-        The expected usernames are those provided as either the user or sysop
-        parameter at instantiation.
+        The expected usernames are those provided as the user parameter
+        at instantiation.
 
         @rtype: bool
         """
         if not hasattr(self, '_userinfo'):
             return False
+        if 'anon' in self.userinfo or not self.userinfo.get('id'):
+            return False
 
-        if 'name' not in self.userinfo or not self.userinfo['name']:
+        if not self.userinfo.get('name'):
             return False
 
         if self.userinfo['name'] != self.username():
@@ -2137,6 +2139,9 @@
             assert 'userinfo' in uidata['query'], \
                    "API userinfo response lacks 'userinfo' key"
             self._userinfo = uidata['query']['userinfo']
+            if 'anon' in self._userinfo or not self._userinfo.get('id'):
+                pywikibot.warning('No user is logged in on site {}'
+                                  .format(self))
         return self._userinfo
 
     userinfo = property(fget=getuserinfo, doc=getuserinfo.__doc__)
@@ -2183,7 +2188,7 @@
 
         @rtype: bool
         """
-        return 'blockinfo' in self._userinfo
+        return 'blockinfo' in self.userinfo
 
     @deprecated('has_right() or is_blocked()', since='20141218')
     @remove_last_args(['sysop'])
@@ -2279,7 +2284,7 @@
 
         U{https://www.mediawiki.org/wiki/API:Userinfo}
         """
-        return right.lower() in self._userinfo['rights']
+        return right.lower() in self.userinfo['rights']
 
     @remove_last_args(['sysop'])
     def has_group(self, group):
@@ -2289,12 +2294,12 @@
         but will usually include bot.
         U{https://www.mediawiki.org/wiki/API:Userinfo}
         """
-        return group.lower() in self._userinfo['groups']
+        return group.lower() in self.userinfo['groups']
 
     @remove_last_args(['sysop'])
     def messages(self):
         """Return true if the user has new messages, and false otherwise."""
-        return 'messages' in self._userinfo
+        return 'messages' in self.userinfo
 
     @need_extension('Echo')
     def notifications(self, **kwargs):
diff --git a/tests/dry_site_tests.py b/tests/dry_site_tests.py
index a37454f..a999dd3 100644
--- a/tests/dry_site_tests.py
+++ b/tests/dry_site_tests.py
@@ -30,18 +30,37 @@
     def test_logged_in(self):
         """Test logged_in() method."""
         x = self.get_site()
-
-        x._userinfo = {'name': None, 'groups': []}
+        x._userinfo = {'name': None, 'groups': [], 'id': 1}
         x._username = 'user'
 
-        self.assertFalse(x.logged_in())
+        with self.subTest(variant='name: None'):
+            self.assertFalse(x.logged_in())
 
         x._userinfo['name'] = 'user'
-        self.assertTrue(x.logged_in())
+        with self.subTest(variant='name: user'):
+            self.assertTrue(x.logged_in())
 
+        x._userinfo['name'] = 'otheruser'
+        with self.subTest(variant='name: otheruseer'):
+            self.assertFalse(x.logged_in())
+
+        x._userinfo['id'] = 0
         x._userinfo['name'] = 'user'
+        with self.subTest(variant='id: 0'):
+            self.assertFalse(x.logged_in())
+
+        x._userinfo['id'] = 1
+        with self.subTest(variant='id: 1'):
+            self.assertTrue(x.logged_in())
+
+        x._userinfo['anon'] = ''
+        with self.subTest(variant='anon'):
+            self.assertFalse(x.logged_in())
+
+        del x._userinfo['anon']
         x._userinfo['groups'] = ['sysop']
-        self.assertTrue(x.logged_in())
+        with self.subTest(variant='sysop'):
+            self.assertTrue(x.logged_in())
 
     def test_user_agent(self):
         """Test different variants of user agents."""

To view, visit change 571001. To unsubscribe, or for help writing mail filters, visit settings.