From 7e7b97480f5eea19e33c3c74d1081a9d26fdf84a Mon Sep 17 00:00:00 2001 From: David Stapleton Date: Fri, 21 Sep 2018 14:03:47 +0100 Subject: [PATCH] Add new 'use_spec_url_for_base_path' bool option to spec.CONFIG_DEFAULTS This new option addresses issues arising from 'basePath' not being present in a given spec. According to the 2.0 spec, "If it is not included, the API is served directly under the host. The value MUST start with a leading slash (/)". 10836363 made the SwaggerClient compliant with the spec (basePath is now implicitly set to '/' if field is missing from spec). However, this broke existing code that utilizes the SwaggerClient (see Issue #299). The current (spec-compliant) behaviour remains with this new option as it defaults to False. Overriding this default with 'True' means the path element of the URL used to retrieve the spec will be used instead. --- bravado_core/spec.py | 24 ++++++++++-- tests/conftest.py | 4 ++ tests/spec/Spec/build_test.py | 30 +++++++++++++++ tests/spec/build_api_serving_url_test.py | 48 ++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/bravado_core/spec.py b/bravado_core/spec.py index 0eaae79a..b8b68d38 100644 --- a/bravado_core/spec.py +++ b/bravado_core/spec.py @@ -74,6 +74,12 @@ # Completely dereference $refs to maximize marshaling and unmarshalling performances. # NOTE: this depends on validate_swagger_spec 'internally_dereference_refs': False, + + # What value to assume for basePath if it is missing from the spec (this + # config option is ignored if basePath is present in the spec) + # If True, use the 'path' element of the URL the spec was retrieved from + # If False, set basePath to '/' (conforms to Swagger 2.0 specification) + 'use_spec_url_for_base_path': False } @@ -197,7 +203,12 @@ def build(self): self.resources = build_resources(self) - self.api_url = build_api_serving_url(self.spec_dict, self.origin_url) + build_api_kwargs = {} + if self.config['use_spec_url_for_base_path']: + build_api_kwargs['use_spec_url_for_base_path'] = True + + self.api_url = build_api_serving_url(self.spec_dict, self.origin_url, + **build_api_kwargs) def _force_deref(self, ref_dict): """Dereference ref_dict (if it is indeed a ref) and return what the @@ -380,7 +391,8 @@ def read_file(uri): } -def build_api_serving_url(spec_dict, origin_url=None, preferred_scheme=None): +def build_api_serving_url(spec_dict, origin_url=None, preferred_scheme=None, + use_spec_url_for_base_path=False): """The URL used to service API requests does not necessarily have to be the same URL that was used to retrieve the API spec_dict. @@ -410,6 +422,9 @@ def build_api_serving_url(spec_dict, origin_url=None, preferred_scheme=None): :param spec_dict: the Swagger spec in json-like dict form :param origin_url: the URL from which the spec was retrieved, if any. This is only used in Swagger clients. + :param use_spec_url_for_base_path: only effective when 'basePath' is missing + from `spec_dict`. When True, 'basePath' will be set to the path portion + of `origin_url`. When False, 'basePath' will be set to '/'. :param preferred_scheme: preferred scheme to use if more than one scheme is supported by the API. :return: base url which services api requests @@ -435,6 +450,9 @@ def pick_a_scheme(schemes): return schemes[0] netloc = spec_dict.get('host', origin.netloc) - path = spec_dict.get('basePath', '/') + base_path = '/' + if use_spec_url_for_base_path: + base_path = origin.path + path = spec_dict.get('basePath', base_path) scheme = pick_a_scheme(spec_dict.get('schemes')) return urlunparse((scheme, netloc, path, None, None, None)) diff --git a/tests/conftest.py b/tests/conftest.py index 9588db51..aecf952f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,6 +28,10 @@ def get_url(absolute_path): return urlparse.urljoin('file:', absolute_path) +def get_url_path(absolute_url): + return urlparse.urlparse(absolute_url).path + + @pytest.fixture def my_dir(): return os.path.abspath(os.path.dirname(__file__)) diff --git a/tests/spec/Spec/build_test.py b/tests/spec/Spec/build_test.py index a6e2bd2d..c29c07bc 100644 --- a/tests/spec/Spec/build_test.py +++ b/tests/spec/Spec/build_test.py @@ -7,6 +7,7 @@ from bravado_core.spec import Spec from tests.conftest import get_url +from tests.conftest import get_url_path from tests.validate.conftest import email_address_format @@ -69,6 +70,35 @@ def test_build_with_internally_dereference_refs(petstore_abspath, petstore_dict, assert (spec.deref == spec._force_deref) == (not internally_dereference_refs) +@pytest.mark.parametrize( + 'use_spec_url_for_base_path', + [ + True, + False, + ] +) +def test_build_using_spec_url_for_base_path(petstore_abspath, petstore_dict, use_spec_url_for_base_path): + # use_spec_url_for_base_path is only effective when basePath is not present + # in the spec, so remove it + del petstore_dict['basePath'] + + origin_url = get_url(petstore_abspath) + spec = Spec( + petstore_dict, + origin_url=origin_url, + config={'use_spec_url_for_base_path': use_spec_url_for_base_path} + ) + + spec.build() + + base_url = 'http://' + petstore_dict['host'] + if not use_spec_url_for_base_path: + assert spec.api_url == base_url + '/' + else: + petstore_path = get_url_path(origin_url) + assert spec.api_url == base_url + petstore_path + + def test_not_object_x_models_are_not_generating_models(minimal_swagger_dict): minimal_swagger_dict['definitions']['Pets'] = { 'type': 'array', diff --git a/tests/spec/build_api_serving_url_test.py b/tests/spec/build_api_serving_url_test.py index 48874b92..92986ae5 100644 --- a/tests/spec/build_api_serving_url_test.py +++ b/tests/spec/build_api_serving_url_test.py @@ -27,6 +27,54 @@ def test_override_basepath(origin_url): assert 'http://www.foo.com:80/v1' == api_serving_url +def test_use_spec_url_True(): + api_serving_url = build_api_serving_url({}, + use_spec_url_for_base_path=True) + assert 'http://localhost/' == api_serving_url + + +def test_use_spec_url_True_when_basePath_present(): + api_serving_url = build_api_serving_url({'basePath': '/v1'}, + use_spec_url_for_base_path=True) + assert 'http://localhost/v1' == api_serving_url + + +def test_use_spec_url_True_when_origin_url_present(origin_url): + api_serving_url = build_api_serving_url({}, origin_url, + use_spec_url_for_base_path=True) + assert 'http://www.foo.com:80/bar/api-docs' == api_serving_url + + +def test_use_spec_url_True_when_basePath_and_origin_url_present(origin_url): + api_serving_url = build_api_serving_url({'basePath': '/v1'}, origin_url, + use_spec_url_for_base_path=True) + assert 'http://www.foo.com:80/v1' == api_serving_url + + +def test_use_spec_url_False(): + api_serving_url = build_api_serving_url({}, + use_spec_url_for_base_path=False) + assert 'http://localhost/' == api_serving_url + + +def test_use_spec_url_False_when_basePath_present(): + api_serving_url = build_api_serving_url({'basePath': '/v1'}, + use_spec_url_for_base_path=False) + assert 'http://localhost/v1' == api_serving_url + + +def test_use_spec_url_False_when_origin_url_present(origin_url): + api_serving_url = build_api_serving_url({}, origin_url, + use_spec_url_for_base_path=False) + assert 'http://www.foo.com:80/' == api_serving_url + + +def test_use_spec_url_False_when_basePath_and_origin_url_present(origin_url): + api_serving_url = build_api_serving_url({'basePath': '/v1'}, origin_url, + use_spec_url_for_base_path=True) + assert 'http://www.foo.com:80/v1' == api_serving_url + + def test_override_scheme(origin_url): spec = {'schemes': ['https']} api_serving_url = build_api_serving_url(spec, origin_url)