Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ibm_cloud_sdk_core/base_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ def set_service_url(self, service_url: str) -> None:
'The service url shouldn\'t start or end with curly brackets or quotes. '
'Be sure to remove any {} and \" characters surrounding your service url'
)
if service_url is not None:
service_url = service_url.rstrip('/')
self.service_url = service_url

def get_http_client(self) -> requests.sessions.Session:
Expand Down Expand Up @@ -368,6 +370,10 @@ def prepare_request(self,
# validate the service url is set
if not self.service_url:
raise ValueError('The service_url is required')

# Combine the service_url and operation path to form the request url.
# Note: we have already stripped any trailing slashes from the service_url
# and we know that the operation path ('url') will start with a slash.
request['url'] = strip_extra_slashes(self.service_url + url)

headers = remove_null_values(headers) if headers else {}
Expand Down
70 changes: 43 additions & 27 deletions test/test_base_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from ibm_cloud_sdk_core import get_authenticator_from_environment
from ibm_cloud_sdk_core.authenticators import (IAMAuthenticator, NoAuthAuthenticator, Authenticator,
BasicAuthenticator, CloudPakForDataAuthenticator)
from ibm_cloud_sdk_core.utils import strip_extra_slashes


class IncludeExternalConfigService(BaseService):
Expand Down Expand Up @@ -698,6 +697,7 @@ def test_user_agent_header():
assert response.get_result().request.headers.__getitem__(
'user-agent') == user_agent_header['User-Agent']


@responses.activate
def test_reserved_keys(caplog):
service = AnyServiceV1('2021-07-02', authenticator=NoAuthAuthenticator())
Expand All @@ -723,6 +723,7 @@ def test_reserved_keys(caplog):
assert caplog.record_tuples[2][2] == '"headers" has been removed from the request'
assert caplog.record_tuples[3][2] == '"cookies" has been removed from the request'


@responses.activate
def test_ssl_error():
responses.add(
Expand Down Expand Up @@ -810,56 +811,71 @@ def test_json():
service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator())
req = service.prepare_request('POST', url='', headers={
'X-opt-out': True}, data={'hello': 'world', 'fóó': 'bår'})
assert req.get('data') == b'{"hello": "world", "f\\u00f3\\u00f3": "b\\u00e5r"}'
assert req.get(
'data') == b'{"hello": "world", "f\\u00f3\\u00f3": "b\\u00e5r"}'


def test_trailing_slash():
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this to test_utils.py

assert strip_extra_slashes('') == ''
assert strip_extra_slashes('//') == '/'
assert strip_extra_slashes('/////') == '/'
assert strip_extra_slashes('https://host') == 'https://host'
assert strip_extra_slashes('https://host/') == 'https://host/'
assert strip_extra_slashes('https://host//') == 'https://host/'
assert strip_extra_slashes('https://host/path') == 'https://host/path'
assert strip_extra_slashes('https://host/path/') == 'https://host/path/'
assert strip_extra_slashes('https://host/path//') == 'https://host/path/'
assert strip_extra_slashes('https://host//path//') == 'https://host//path/'

def test_service_url_handling():
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the direct calls to strip_extra_slashes() so this test will focus on the service and service_url

service = AnyServiceV1(
'2018-11-20', service_url='https://host/', authenticator=NoAuthAuthenticator())
assert service.service_url == 'https://host/'
'2018-11-20', service_url='https://host///////', authenticator=NoAuthAuthenticator())
assert service.service_url == 'https://host'

service.set_service_url('https://host/')
assert service.service_url == 'https://host/'
assert service.service_url == 'https://host'

req = service.prepare_request('POST',
url='/path/',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert req.get('url') == 'https://host//path/'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can no longer have double-slashes between the service_url and the operation path

assert req.get('url') == 'https://host/path/'

service = AnyServiceV1(
'2018-11-20', service_url='https://host/', authenticator=NoAuthAuthenticator())
assert service.service_url == 'https://host/'
assert service.service_url == 'https://host'

service.set_service_url('https://host/')
assert service.service_url == 'https://host/'
assert service.service_url == 'https://host'

req = service.prepare_request('POST',
url='/',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert req.get('url') == 'https://host/'

req = service.prepare_request('POST',
url='////',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert req.get('url') == 'https://host/'

service.set_service_url(None)
assert service.service_url is None

service = AnyServiceV1('2018-11-20', service_url='/',
authenticator=NoAuthAuthenticator())
assert service.service_url == '/'
assert service.service_url == ''

service.set_service_url('/')
assert service.service_url == '/'
req = service.prepare_request('POST',
url='/',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert req.get('url') == '/'
assert service.service_url == ''

with pytest.raises(ValueError) as err:
service.prepare_request('POST',
url='/',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert str(err.value) == 'The service_url is required'


def test_service_url_slash():
service = AnyServiceV1('2018-11-20', service_url='/',
authenticator=NoAuthAuthenticator())
assert service.service_url == ''
with pytest.raises(ValueError) as err:
service.prepare_request('POST',
url='/',
headers={'X-opt-out': True},
data={'hello': 'world'})
assert str(err.value) == 'The service_url is required'


def test_service_url_not_set():
Expand Down
16 changes: 16 additions & 0 deletions test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from ibm_cloud_sdk_core import get_query_param
from ibm_cloud_sdk_core import read_external_sources
from ibm_cloud_sdk_core.authenticators import Authenticator, BasicAuthenticator, IAMAuthenticator
from ibm_cloud_sdk_core.utils import strip_extra_slashes


def datetime_test(datestr: str, expected: str):
Expand Down Expand Up @@ -616,3 +617,18 @@ def test_read_external_sources_2():

config = read_external_sources('service_1')
assert config.get('URL') == 'service1.com/api'


def test_strip_extra_slashes():
assert strip_extra_slashes('') == ''
assert strip_extra_slashes('//') == '/'
assert strip_extra_slashes('/////') == '/'
assert strip_extra_slashes('https://host') == 'https://host'
assert strip_extra_slashes('https://host/') == 'https://host/'
assert strip_extra_slashes('https://host//') == 'https://host/'
assert strip_extra_slashes('https://host/path') == 'https://host/path'
assert strip_extra_slashes('https://host/path/') == 'https://host/path/'
assert strip_extra_slashes('https://host/path//') == 'https://host/path/'
assert strip_extra_slashes('https://host//path//') == 'https://host//path/'
assert strip_extra_slashes(
'https://host//path//////////') == 'https://host//path/'