From 70675997e1a7669564912250fde5c35ea2c0a0f8 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Mon, 21 Sep 2015 18:27:42 -0700 Subject: [PATCH] Use requests in http check instead of urllib The requests interface is much nicer and easier to use so we might as well use it instead of direct urllib usage. Change-Id: I364ddb5f86900a3e166f4480d9f4889a68de247f --- oslo_policy/_checks.py | 11 +++--- oslo_policy/tests/test_checks.py | 61 ++++++++++++++++++-------------- requirements.txt | 1 + test-requirements.txt | 1 + 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/oslo_policy/_checks.py b/oslo_policy/_checks.py index b011294b..16b665a3 100644 --- a/oslo_policy/_checks.py +++ b/oslo_policy/_checks.py @@ -14,14 +14,15 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. + import abc import ast +import contextlib import copy from oslo_serialization import jsonutils +import requests import six -import six.moves.urllib.parse as urlparse -import six.moves.urllib.request as urlrequest registered_checks = {} @@ -268,12 +269,10 @@ class HttpCheck(Check): element = target.get(key) if type(element) is object: temp_target[key] = {} - data = {'target': jsonutils.dumps(temp_target), 'credentials': jsonutils.dumps(creds)} - post_data = urlparse.urlencode(data) - f = urlrequest.urlopen(url, post_data) - return f.read() == 'True' + with contextlib.closing(requests.post(url, data=data)) as r: + return r.text == 'True' @register(None) diff --git a/oslo_policy/tests/test_checks.py b/oslo_policy/tests/test_checks.py index 31144c49..fbc7a732 100644 --- a/oslo_policy/tests/test_checks.py +++ b/oslo_policy/tests/test_checks.py @@ -13,13 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. - +import httpretty import mock from oslo_serialization import jsonutils from oslotest import base as test_base -import six import six.moves.urllib.parse as urlparse -import six.moves.urllib.request as urlrequest from oslo_policy import _checks from oslo_policy.tests import base @@ -88,47 +86,52 @@ class HttpCheckTestCase(base.PolicyBaseTestCase): for item in post_data.split('&'): key, _sep, value = item.partition('=') result[key] = jsonutils.loads(urlparse.unquote_plus(value)) - return result - @mock.patch.object(urlrequest, 'urlopen', - return_value=six.StringIO('True')) - def test_accept(self, mock_urlopen): + @httpretty.activate + def test_accept(self): + httpretty.register_uri(httpretty.POST, + "http://example.com/target", + body='True') + httpretty.HTTPretty.allow_net_connect = False + check = _checks.HttpCheck('http', '//example.com/%(name)s') self.assertTrue(check(dict(name='target', spam='spammer'), dict(user='user', roles=['a', 'b', 'c']), self.enforcer)) - self.assertEqual(1, mock_urlopen.call_count) - args = mock_urlopen.call_args[0] - - self.assertEqual('http://example.com/target', args[0]) + last_request = httpretty.last_request() + self.assertEqual('POST', last_request.method) self.assertEqual(dict( target=dict(name='target', spam='spammer'), credentials=dict(user='user', roles=['a', 'b', 'c']), - ), self.decode_post_data(args[1])) + ), self.decode_post_data(last_request.body.decode("utf8"))) + + @httpretty.activate + def test_reject(self): + httpretty.register_uri(httpretty.POST, + "http://example.com/target", + body='other') + httpretty.HTTPretty.allow_net_connect = False - @mock.patch.object(urlrequest, 'urlopen', - return_value=six.StringIO('other')) - def test_reject(self, mock_urlopen): check = _checks.HttpCheck('http', '//example.com/%(name)s') - self.assertFalse(check(dict(name='target', spam='spammer'), dict(user='user', roles=['a', 'b', 'c']), self.enforcer)) - self.assertEqual(1, mock_urlopen.call_count) - args = mock_urlopen.call_args[0] - - self.assertEqual('http://example.com/target', args[0]) + last_request = httpretty.last_request() + self.assertEqual('POST', last_request.method) self.assertEqual(dict( target=dict(name='target', spam='spammer'), credentials=dict(user='user', roles=['a', 'b', 'c']), - ), self.decode_post_data(args[1])) + ), self.decode_post_data(last_request.body.decode("utf8"))) - @mock.patch.object(urlrequest, 'urlopen', - return_value=six.StringIO('True')) - def test_http_with_objects_in_target(self, mock_urlopen): + @httpretty.activate + def test_http_with_objects_in_target(self): + httpretty.register_uri(httpretty.POST, + "http://example.com/target", + body='True') + httpretty.HTTPretty.allow_net_connect = False check = _checks.HttpCheck('http', '//example.com/%(name)s') target = {'a': object(), @@ -138,9 +141,13 @@ class HttpCheckTestCase(base.PolicyBaseTestCase): dict(user='user', roles=['a', 'b', 'c']), self.enforcer)) - @mock.patch.object(urlrequest, 'urlopen', - return_value=six.StringIO('True')) - def test_http_with_strings_in_target(self, mock_urlopen): + @httpretty.activate + def test_http_with_strings_in_target(self): + httpretty.register_uri(httpretty.POST, + "http://example.com/target", + body='True') + httpretty.HTTPretty.allow_net_connect = False + check = _checks.HttpCheck('http', '//example.com/%(name)s') target = {'a': 'some_string', 'name': 'target', diff --git a/requirements.txt b/requirements.txt index c640c36e..f5891a13 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. +requests>=2.5.2 oslo.config>=2.3.0 # Apache-2.0 oslo.i18n>=1.5.0 # Apache-2.0 oslo.serialization>=1.4.0 # Apache-2.0 diff --git a/test-requirements.txt b/test-requirements.txt index e7442c01..696d7495 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,6 +4,7 @@ hacking<0.11,>=0.10.0 oslotest>=1.10.0 # Apache-2.0 +httpretty>=0.8.4,<0.8.7 # These are needed for docs generation oslosphinx>=2.5.0 # Apache-2.0