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

Change subject: [FIX] Request: Ensure that keys are ASCII ......................................................................

[FIX] Request: Ensure that keys are ASCII

When in Python 2 the keys are defined via explicit arguments they are `bytes` while they can be `unicode` instances when added later or using the `parameters` parameter or the **-notation.

This is encoding the key using ASCII on Python 2 to normalize the keys and making the cache easier to compare two requests. If requests with the same parameters where created using different methods it may not use the same cache name for all of them.

In Python 3 it's just checking that it's possible encode the keys using ASCII bot not actually encode it as by default keyword arguments are also named using `unicode` instances.

The `tests.dry_api_tests.DryCachedRequestTests` are now using the parameters notation (which was added in b44e59ae) together with having the arguments explicitly added and using the **-notation.

Change-Id: Idd3ce9588510295ca5742fb31f27ab219f3d2426 --- M pywikibot/data/api.py M tests/dry_api_tests.py 2 files changed, 48 insertions(+), 7 deletions(-)

Approvals: John Vandenberg: Looks good to me, approved jenkins-bot: Verified

diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py index a7b7f6d..d838f0c 100644 --- a/pywikibot/data/api.py +++ b/pywikibot/data/api.py @@ -1736,6 +1736,11 @@ pywikibot.error( u"_encoded_items: '%s' could not be encoded as '%s':" u" %r" % (key, self.site.encoding(), value)) + if PY2: + key = key.encode('ascii') + else: + assert key.encode('ascii') + assert isinstance(key, str) params[key] = value return params

diff --git a/tests/dry_api_tests.py b/tests/dry_api_tests.py index 5266256..6319154 100644 --- a/tests/dry_api_tests.py +++ b/tests/dry_api_tests.py @@ -48,21 +48,49 @@

def setUp(self): super(DryCachedRequestTests, self).setUp() - self.parms = {'site': self.basesite, - 'action': 'query', + self.parms = {'action': 'query', 'meta': 'userinfo'} - self.req = CachedRequest(expiry=1, **self.parms) - self.expreq = CachedRequest(expiry=0, **self.parms) - self.diffreq = CachedRequest(expiry=1, site=self.basesite, action='query', meta='siteinfo') - self.diffsite = CachedRequest(expiry=1, site=self.altsite, action='query', meta='userinfo') + self.req = CachedRequest(expiry=1, site=self.basesite, + parameters=self.parms) + self.expreq = CachedRequest(expiry=0, site=self.basesite, + parameters=self.parms) + self.diffreq = CachedRequest( + expiry=1, site=self.basesite, + parameters={'action': 'query', 'meta': 'siteinfo'}) + self.diffsite = CachedRequest( + expiry=1, site=self.altsite, + parameters={'action': 'query', 'meta': 'userinfo'}) + # When using ** the paramters are still unicode + self.deprecated_explicit = CachedRequest( + expiry=1, site=self.basesite, action='query', meta='userinfo') + self.deprecated_asterisks = CachedRequest( + expiry=1, site=self.basesite, **self.parms)

def test_expiry_formats(self): self.assertEqual(self.req.expiry, - CachedRequest(datetime.timedelta(days=1), **self.parms).expiry) + CachedRequest(datetime.timedelta(days=1), site=self.basesite, + parameters=self.parms).expiry)

def test_expired(self): self.assertFalse(self.req._expired(datetime.datetime.now())) self.assertTrue(self.req._expired(datetime.datetime.now() - datetime.timedelta(days=2))) + + def test_parameter_types(self): + """Test _uniquedescriptionstr is identical using different ways.""" + # This test is done as create_file_name and cachefile_path only use + # the hashed name which is not very helpful + self.assertEqual(self.req._uniquedescriptionstr(), + self.req._uniquedescriptionstr()) + self.assertEqual(self.req._uniquedescriptionstr(), + self.expreq._uniquedescriptionstr()) + self.assertEqual(self.req._uniquedescriptionstr(), + self.deprecated_explicit._uniquedescriptionstr()) + self.assertEqual(self.req._uniquedescriptionstr(), + self.deprecated_asterisks._uniquedescriptionstr()) + self.assertNotEqual(self.req._uniquedescriptionstr(), + self.diffreq._uniquedescriptionstr()) + self.assertNotEqual(self.req._uniquedescriptionstr(), + self.diffsite._uniquedescriptionstr())

def test_get_cache_dir(self): retval = self.req._get_cache_dir() @@ -71,11 +99,19 @@ def test_create_file_name(self): self.assertEqual(self.req._create_file_name(), self.req._create_file_name()) self.assertEqual(self.req._create_file_name(), self.expreq._create_file_name()) + self.assertEqual(self.req._create_file_name(), + self.deprecated_explicit._create_file_name()) + self.assertEqual(self.req._create_file_name(), + self.deprecated_asterisks._create_file_name()) self.assertNotEqual(self.req._create_file_name(), self.diffreq._create_file_name())

def test_cachefile_path(self): self.assertEqual(self.req._cachefile_path(), self.req._cachefile_path()) self.assertEqual(self.req._cachefile_path(), self.expreq._cachefile_path()) + self.assertEqual(self.req._cachefile_path(), + self.deprecated_explicit._cachefile_path()) + self.assertEqual(self.req._cachefile_path(), + self.deprecated_asterisks._cachefile_path()) self.assertNotEqual(self.req._cachefile_path(), self.diffreq._cachefile_path()) self.assertNotEqual(self.req._cachefile_path(), self.diffsite._cachefile_path())