jenkins-bot merged this change.
pywikibot.site.Siteinfo: Fix the bug in cache_time when loading a CachedRequest
Siteinfo._post_process:
- Make _post_process a static method (self was not used)
Siteinfo._get_siteinfo:
- Make the try block narrower. Only request.submit raises APIError.
- If the request was a CachedRequest, use the _cachetime attribute
as cache_time. This will resolve T202227.
Note that the _cachetime attribute is not UTC. In order to be
compatible we will need to change CachedRequest to always use
UTC time.
Siteinfo.get:
- The cache in the except clause was not used. Replace it with a `pass`.
api.CachedRequest:
- Always use UTC timestamps. (see the explanation of
Siteinfo._get_siteinfo)
api_tests.py, dry_api_tests.py, cache.py:
- Update code related to api.CachedRequest to use utcnow.
site_tests.py:
- Add a dry test for this fix. The result of this test used to
contain the value of utcnow() instead of '_cache_time'.
tests.__init__:
Import MagicMock and add it to __all__ so that it can be used more
conveniently.
Bug: T202227
Change-Id: Ic5fe36b3a2440c3e7fdf6837996d0b162d7643c7
---
M pywikibot/data/api.py
M pywikibot/site.py
M scripts/maintenance/cache.py
M tests/__init__.py
M tests/api_tests.py
M tests/dry_api_tests.py
M tests/site_tests.py
7 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py
index 2acf7fb..9adc8f5 100644
--- a/pywikibot/data/api.py
+++ b/pywikibot/data/api.py
@@ -2389,7 +2389,7 @@
self._create_file_name())
def _expired(self, dt):
- return dt + self.expiry < datetime.datetime.now()
+ return dt + self.expiry < datetime.datetime.utcnow()
def _load_cache(self):
"""Load cache entry for request, if available.
@@ -2419,7 +2419,7 @@
def _write_cache(self, data):
"""Write data to self._cachefile_path()."""
- data = [self._uniquedescriptionstr(), data, datetime.datetime.now()]
+ data = (self._uniquedescriptionstr(), data, datetime.datetime.utcnow())
with open(self._cachefile_path(), 'wb') as f:
pickle.dump(data, f, protocol=config.pickle_protocol)
diff --git a/pywikibot/site.py b/pywikibot/site.py
index 0adfbcc..0177c0e 100644
--- a/pywikibot/site.py
+++ b/pywikibot/site.py
@@ -1456,7 +1456,8 @@
else:
return pywikibot.tools.EMPTY_DEFAULT
- def _post_process(self, prop, data):
+ @staticmethod
+ def _post_process(prop, data):
"""Do some default handling of data. Directly modifies data."""
# Be careful with version tests inside this here as it might need to
# query this method to actually get the version number
@@ -1523,19 +1524,19 @@
if len(props) == 0:
raise ValueError('At least one property name must be provided.')
invalid_properties = []
+ request = self._site._request(
+ expiry=pywikibot.config.API_config_expiry
+ if expiry is False else expiry,
+ parameters={
+ 'action': 'query', 'meta': 'siteinfo', 'siprop': props,
+ }
+ )
+ # With 1.25wmf5 it'll require continue or rawcontinue. As we don't
+ # continue anyway we just always use continue.
+ request['continue'] = True
+ # warnings are handled later
+ request._warning_handler = warn_handler
try:
- request = self._site._request(
- expiry=pywikibot.config.API_config_expiry
- if expiry is False else expiry,
- parameters={
- 'action': 'query', 'meta': 'siteinfo', 'siprop': props,
- }
- )
- # With 1.25wmf5 it'll require continue or rawcontinue. As we don't
- # continue anyway we just always use continue.
- request['continue'] = True
- # warnings are handled later
- request._warning_handler = warn_handler
data = request.submit()
except api.APIError as e:
if e.code == 'siunknown_siprop':
@@ -1561,7 +1562,9 @@
pywikibot.log(u"Unable to get siprop(s) '{0}'".format(
u"', '".join(invalid_properties)))
if 'query' in data:
- cache_time = datetime.datetime.utcnow()
+ # If the request is a CachedRequest, use the _cachetime attr.
+ cache_time = getattr(
+ request, '_cachetime', None) or datetime.datetime.utcnow()
for prop in props:
if prop in data['query']:
self._post_process(prop, data['query'][prop])
@@ -1658,7 +1661,7 @@
try:
cached = self._get_cached(key)
except KeyError:
- cached = None
+ pass
else: # cached value available
# is a default value, but isn't accepted
if not cached[1] and not get_default:
diff --git a/scripts/maintenance/cache.py b/scripts/maintenance/cache.py
index 27752da..6eb9085 100755
--- a/scripts/maintenance/cache.py
+++ b/scripts/maintenance/cache.py
@@ -348,13 +348,13 @@
def older_than(entry, interval):
"""Find older entries."""
- if entry._cachetime + interval < datetime.datetime.now():
+ if entry._cachetime + interval < datetime.datetime.utcnow():
return entry
def newer_than(entry, interval):
"""Find newer entries."""
- if entry._cachetime + interval >= datetime.datetime.now():
+ if entry._cachetime + interval >= datetime.datetime.utcnow():
return entry
diff --git a/tests/__init__.py b/tests/__init__.py
index 650ccac..39a1651 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -7,8 +7,9 @@
#
from __future__ import absolute_import, print_function, unicode_literals
-__all__ = ('requests', 'unittest', 'TestRequest',
- 'patch_request', 'unpatch_request', 'mock', 'Mock', 'patch')
+__all__ = (
+ 'requests', 'unittest', 'TestRequest', 'patch_request', 'unpatch_request',
+ 'mock', 'Mock', 'MagicMock', 'patch')
import functools
import os
@@ -35,10 +36,10 @@
import unittest
try:
import unittest.mock as mock
- from unittest.mock import Mock, patch
+ from unittest.mock import MagicMock, Mock, patch
except ImportError:
import mock
- from mock import Mock, patch
+ from mock import MagicMock, Mock, patch
_root_dir = os.path.split(os.path.split(__file__)[0])[0]
diff --git a/tests/api_tests.py b/tests/api_tests.py
index 0a04e61..7e0d9b6 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -933,7 +933,7 @@
mysite = self.get_site()
# Run tests on a missing page unique to this test run so it can
# not be cached the first request, but will be cached after.
- now = datetime.datetime.now()
+ now = datetime.datetime.utcnow()
params = {'action': 'query',
'prop': 'info',
'titles': 'TestCachedRequest_test_internals ' + str(now),
diff --git a/tests/dry_api_tests.py b/tests/dry_api_tests.py
index 2b2f206..b0ec916 100644
--- a/tests/dry_api_tests.py
+++ b/tests/dry_api_tests.py
@@ -73,8 +73,9 @@
def test_expired(self):
"""Test if the request is expired."""
- self.assertFalse(self.req._expired(datetime.datetime.now()))
- self.assertTrue(self.req._expired(datetime.datetime.now() - datetime.timedelta(days=2)))
+ self.assertFalse(self.req._expired(datetime.datetime.utcnow()))
+ self.assertTrue(self.req._expired(
+ datetime.datetime.utcnow() - datetime.timedelta(days=2)))
def test_parameter_types(self):
"""Test _uniquedescriptionstr is identical using different ways."""
diff --git a/tests/site_tests.py b/tests/site_tests.py
index 01ae4b0..cf7a9b4 100644
--- a/tests/site_tests.py
+++ b/tests/site_tests.py
@@ -34,7 +34,7 @@
UnicodeType as unicode,
)
-from tests import patch, unittest_print
+from tests import patch, unittest_print, MagicMock
from tests.aspects import (
unittest, TestCase, DeprecationTestCase,
TestCaseBase,
@@ -2414,6 +2414,22 @@
self.assertFalse(entered_loop(mysite.siteinfo.get(not_exists).keys()))
+class TestSiteinfoDry(DefaultDrySiteTestCase):
+
+ """Test Siteinfo in dry mode."""
+
+ def test_siteinfo_timestamps(self):
+ """Test that cache has the timestamp of CachedRequest."""
+ site = self.get_site()
+ request_mock = MagicMock()
+ request_mock.submit = lambda: {'query': {'_prop': '_value'}}
+ request_mock._cachetime = '_cache_time'
+ with patch.object(site, '_request', return_value=request_mock):
+ siteinfo = pywikibot.site.Siteinfo(site)
+ result = siteinfo._get_siteinfo('_prop', False)
+ self.assertEqual(result, {'_prop': ('_value', '_cache_time')})
+
+
class TestSiteinfoAsync(DefaultSiteTestCase):
"""Test asynchronous siteinfo fetch."""
To view, visit change 453944. To unsubscribe, or for help writing mail filters, visit settings.