Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uplink.Url by checking url type (fixes #164) #165

Merged
merged 4 commits into from
Aug 18, 2019
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
13 changes: 13 additions & 0 deletions tests/integration/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ def list_repos(self, user):
def get_repo(self, user, repo):
pass

@uplink.get(args={"url": uplink.Url})
def forward(self, url):
pass


def test_list_repo(mock_client):
github = GitHubService(base_url=BASE_URL, client=mock_client)
Expand Down Expand Up @@ -51,6 +55,15 @@ def test_get_repo(mock_client, mock_response):
assert expected_json == actual_json


def test_forward(mock_client):
github = GitHubService(base_url=BASE_URL, client=mock_client)
github.forward("/users/prkumar/repos")
request = mock_client.history[0]
assert request.method == "GET"
assert request.has_base_url(BASE_URL)
assert request.has_endpoint("/users/prkumar/repos")


def test_handle_client_exceptions(mock_client):
# Setup: mock client exceptions

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def test_modify_request_definition(self, request_definition_builder):

def test_modify_request(self, request_builder):
arguments.Path("name").modify_request(request_builder, "value")
request_builder.url.set_variable.assert_called_with({"name": "value"})
request_builder.set_url_variable.assert_called_with({"name": "value"})


class TestQuery(ArgumentTestCase, FuncDecoratorTestCase):
Expand Down Expand Up @@ -421,7 +421,7 @@ def test_modify_request_definition_failure(

def test_modify_request(self, request_builder):
arguments.Url().modify_request(request_builder, "/some/path")
assert request_builder.url == "/some/path"
assert request_builder.relative_url == "/some/path"


class TestTimeout(ArgumentTestCase, FuncDecoratorTestCase):
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def uplink_builder(http_client_mock):
class TestRequestPreparer(object):
def test_prepare_request(self, mocker, request_builder):
request_builder.method = "METHOD"
request_builder.url = "/example/path"
request_builder.relative_url = "/example/path"
request_builder.return_type = None
request_builder.transaction_hooks = ()
request_builder.request_template = "request_template"
Expand All @@ -45,7 +45,7 @@ def test_prepare_request_with_transaction_hook(
self, mocker, uplink_builder, request_builder, transaction_hook_mock
):
request_builder.method = "METHOD"
request_builder.url = "/example/path"
request_builder.relative_url = "/example/path"
request_builder.request_template = "request_template"
uplink_builder.base_url = "https://example.com"
request_builder.transaction_hooks = [transaction_hook_mock]
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def test_define_request(self, request_builder, mocker):
)
definition.define_request(request_builder, (), {})
assert request_builder.method == method
assert request_builder.url == uri
assert request_builder.relative_url == uri
assert request_builder.return_type is str

def test_make_converter_registry(self, annotation_handler_mock):
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/test_helpers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import pytest

# Local imports
from uplink import helpers


Expand Down Expand Up @@ -53,3 +56,22 @@ def test_context(self):

# Verify
assert builder.context["key"] == "value"

def test_relative_url_template(self):
# Setup
builder = helpers.RequestBuilder(None, {}, "base_url")

# Run
builder.relative_url = "/v1/api/users/{username}/repos"
builder.set_url_variable({"username": "cognifloyd"})

# Verify
assert builder.relative_url == "/v1/api/users/cognifloyd/repos"

def test_relative_url_template_type_error(self):
# Setup
builder = helpers.RequestBuilder(None, {}, "base_url")

# Run
with pytest.raises(TypeError):
builder.relative_url = 1
5 changes: 3 additions & 2 deletions uplink/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def modify_request_definition(self, request_definition_builder):
request_definition_builder.uri.add_variable(self.name)

def _modify_request(self, request_builder, value):
request_builder.url.set_variable({self.name: value})
request_builder.set_url_variable({self.name: value})


class Query(FuncDecoratorMixin, NamedArgument):
Expand Down Expand Up @@ -639,6 +639,7 @@ def _modify_request(self, request_builder, value):
request_builder.info["data"] = value


# TODO: Add an integration test that uses arguments.Url
class Url(ArgumentAnnotation):
"""
Sets a dynamic URL.
Expand Down Expand Up @@ -679,7 +680,7 @@ def modify_request_definition(self, request_definition_builder):
@classmethod
def _modify_request(cls, request_builder, value):
"""Updates request url."""
request_builder.url = value
request_builder.relative_url = value


class Timeout(FuncDecoratorMixin, ArgumentAnnotation):
Expand Down
4 changes: 0 additions & 4 deletions uplink/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ def __init__(self, builder, consumer=None):
else:
self._session_chain = None

def _join_url_with_base(self, url):
return utils.urlparse.urljoin(self._base_url, url)

@staticmethod
def _get_request_hooks(contract):
chain = list(contract.transaction_hooks)
Expand All @@ -62,7 +59,6 @@ def apply_hooks(self, execution_builder, chain):
execution_builder.with_errbacks(self._wrap_hook(chain.handle_exception))

def prepare_request(self, request_builder, execution_builder):
request_builder.url = self._join_url_with_base(request_builder.url)
self._auth(request_builder)
request_hooks = self._get_request_hooks(request_builder)
if request_hooks:
Expand Down
3 changes: 1 addition & 2 deletions uplink/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,12 @@ def make_converter_registry(self, converters_):

def define_request(self, request_builder, func_args, func_kwargs):
request_builder.method = self._method
request_builder.url = utils.URIBuilder(self._uri)
request_builder.relative_url = self._uri
request_builder.return_type = self._return_type
self._argument_handler.handle_call(
request_builder, func_args, func_kwargs
)
self._method_handler.handle_builder(request_builder)
request_builder.url = request_builder.url.build()


class HttpMethodFactory(object):
Expand Down
21 changes: 14 additions & 7 deletions uplink/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import collections

# Local imports
from uplink import interfaces
from uplink import interfaces, utils
from uplink.clients import io


Expand Down Expand Up @@ -39,7 +39,7 @@ def set_api_definition(service, name, definition):
class RequestBuilder(object):
def __init__(self, client, converter_registry, base_url):
self._method = None
self._url = None
self._relative_url_template = utils.URIBuilder("")
self._return_type = None
self._client = client
self._base_url = base_url
Expand Down Expand Up @@ -69,13 +69,16 @@ def method(self, method):
def base_url(self):
return self._base_url

def set_url_variable(self, variables):
self._relative_url_template.set_variable(variables)

@property
def url(self):
return self._url
def relative_url(self):
return self._relative_url_template.build()

@url.setter
def url(self, url):
self._url = url
@relative_url.setter
def relative_url(self, url):
self._relative_url_template = utils.URIBuilder(url)

@property
def info(self):
Expand Down Expand Up @@ -104,6 +107,10 @@ def return_type(self, return_type):
def request_template(self):
return io.CompositeRequestTemplate(self._request_templates)

@property
def url(self):
return utils.urlparse.urljoin(self.base_url, self.relative_url)

def add_transaction_hook(self, hook):
self._transaction_hooks.append(hook)

Expand Down