From 3d8d14d762319edde12e8866ee2b7cdea275c3ab Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 13 May 2021 13:55:56 +0200 Subject: [PATCH] Fix best_match() deprecation warning Our WSGI calls best_match() and generates the following warning: DeprecationWarning: The behavior of .best_match for the Accept classes is currently being maintained for backward compatibility, but the method will be deprecated in the future, as its behavior is not specified in (and currently does not conform to) RFC 7231. We can see these when running the api-ref functional tests. See the following URL for more information about deprecation. https://docs.pylonsproject.org/projects/webob/en/stable/api/webob.html#webob.acceptparse.AcceptValidHeader.best_match Fix them as per Takashi's patch on Nova with change id Ib93ffffd3f28aa6d53a0a4eed5aeee94900cb073 (his name has been removed from the NOTE because we no longer do that in Cinder). Closes-Bug: #1798224 Change-Id: I4084bc9c58146fc7a56131dc60c9893cb24539ee --- cinder/api/openstack/wsgi.py | 16 ++++- cinder/tests/unit/api/openstack/test_wsgi.py | 62 ++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index ab9e0273ec8..da18f5a5719 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -240,7 +240,10 @@ class Request(webob.Request): content_type = possible_type if not content_type: - content_type = self.accept.best_match(SUPPORTED_CONTENT_TYPES) + best_matches = self.accept.acceptable_offers( + SUPPORTED_CONTENT_TYPES) + if best_matches: + content_type = best_matches[0][0] self.environ['cinder.best_content_type'] = (content_type or 'application/json') @@ -272,7 +275,16 @@ class Request(webob.Request): if not self.accept_language: return None all_languages = i18n.get_available_languages() - return self.accept_language.best_match(all_languages) + # NOTE: To decide the default behavior, 'default' is preferred over + # 'default_tag' because that is return as it is when no match. This is + # also little tricky that 'default' value cannot be None. At least one + # of default_tag or default must be supplied as an argument to the + # method, to define the defaulting behavior. So passing a sentinal + # value to return None from this function. + best_match = self.accept_language.lookup(all_languages, default='fake') + if best_match == 'fake': + return None + return best_match def set_api_version_request(self, url): """Set API version request based on the request header information. diff --git a/cinder/tests/unit/api/openstack/test_wsgi.py b/cinder/tests/unit/api/openstack/test_wsgi.py index c436ac48a0c..a3c76c6e017 100644 --- a/cinder/tests/unit/api/openstack/test_wsgi.py +++ b/cinder/tests/unit/api/openstack/test_wsgi.py @@ -25,6 +25,12 @@ from cinder.tests.unit import test class RequestTest(test.TestCase): + def setUp(self): + super(RequestTest, self).setUp() + self.patch('cinder.i18n.get_available_languages', + return_value=['en-GB', 'en-AU', 'de', 'zh-CN', 'en-US', + 'ja-JP']) + def test_content_type_missing(self): request = wsgi.Request.blank('/tests/123', method='POST') request.body = b"" @@ -72,6 +78,62 @@ class RequestTest(test.TestCase): result = request.best_match_content_type() self.assertEqual("application/json", result) + def test_content_type_accept_with_quality_values(self): + request = wsgi.Request.blank('/tests/123') + request.headers["Accept"] = ( + "application/json;q=0.4," + "application/vnd.openstack.volume+json;q=0.6") + result = request.best_match_content_type() + self.assertEqual("application/vnd.openstack.volume+json", result) + + def test_from_request(self): + request = wsgi.Request.blank('/') + accepted = 'bogus;q=1, en-gb;q=0.7,en-us,en;q=0.5,*;q=0.7' + request.headers = {'Accept-Language': accepted} + self.assertEqual(request.best_match_language(), 'en-US') + + def test_asterisk(self): + # In the section 3.4 of RFC4647, it says as follows: + # If the language range "*"(asterisk) is the only one + # in the language priority list or if no other language range + # follows, the default value is computed and returned. + # + # In this case, the default value 'None' is returned. + request = wsgi.Request.blank('/') + accepted = '*;q=0.5' + request.headers = {'Accept-Language': accepted} + self.assertIsNone(request.best_match_language()) + + def test_asterisk_followed_by_other_language(self): + request = wsgi.Request.blank('/') + accepted = '*,ja-jp;q=0.5' + request.headers = {'Accept-Language': accepted} + self.assertEqual('ja-JP', request.best_match_language()) + + def test_truncate(self): + request = wsgi.Request.blank('/') + accepted = 'de-CH' + request.headers = {'Accept-Language': accepted} + self.assertEqual('de', request.best_match_language()) + + def test_secondary(self): + request = wsgi.Request.blank('/') + accepted = 'nn,en-gb;q=0.5' + request.headers = {'Accept-Language': accepted} + self.assertEqual('en-GB', request.best_match_language()) + + def test_none_found(self): + request = wsgi.Request.blank('/') + accepted = 'nb-no' + request.headers = {'Accept-Language': accepted} + self.assertIsNone(request.best_match_language()) + + def test_no_lang_header(self): + request = wsgi.Request.blank('/') + accepted = '' + request.headers = {'Accept-Language': accepted} + self.assertIsNone(request.best_match_language()) + def test_best_match_language(self): # Test that we are actually invoking language negotiation by webob request = wsgi.Request.blank('/')