diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py index 821fdada6..7f7fec3ee 100644 --- a/tableauserverclient/server/endpoint/endpoint.py +++ b/tableauserverclient/server/endpoint/endpoint.py @@ -41,9 +41,9 @@ def _safe_to_log(server_response): def _make_request(self, method, url, content=None, request_object=None, auth_token=None, content_type=None, parameters=None): - if request_object is not None: - url = request_object.apply_query_params(url) parameters = parameters or {} + if request_object is not None: + parameters["params"] = request_object.get_query_params() parameters.update(self.parent_srv.http_options) parameters['headers'] = Endpoint._make_common_headers(auth_token, content_type) diff --git a/tableauserverclient/server/request_options.py b/tableauserverclient/server/request_options.py index 530d7d1f0..f94fff90f 100644 --- a/tableauserverclient/server/request_options.py +++ b/tableauserverclient/server/request_options.py @@ -3,6 +3,22 @@ class RequestOptionsBase(object): def apply_query_params(self, url): + import warnings + warnings.simplefilter('always', DeprecationWarning) + warnings.warn('apply_query_params is deprecated, please use get_query_params instead.', DeprecationWarning) + try: + params = self.get_query_params() + params_list = ["{}={}".format(k, v) for (k, v) in params.items()] + + if '?' in url: + url, existing_params = url.split('?') + params_list.append(existing_params) + + return "{0}?{1}".format(url, '&'.join(params_list)) + except NotImplementedError: + raise + + def get_query_params(self): raise NotImplementedError() @@ -61,27 +77,21 @@ def page_number(self, page_number): self.pagenumber = page_number return self - def apply_query_params(self, url): - params = [] - - if '?' in url: - url, existing_params = url.split('?') - params.append(existing_params) - - if self.page_number: - params.append('pageNumber={0}'.format(self.pagenumber)) - if self.page_size: - params.append('pageSize={0}'.format(self.pagesize)) + def get_query_params(self): + params = {} + if self.pagenumber: + params['pageNumber'] = self.pagenumber + if self.pagesize: + params['pageSize'] = self.pagesize if len(self.sort) > 0: sort_options = (str(sort_item) for sort_item in self.sort) ordered_sort_options = sorted(sort_options) - params.append('sort={}'.format(','.join(ordered_sort_options))) + params['sort'] = ','.join(ordered_sort_options) if len(self.filter) > 0: filter_options = (str(filter_item) for filter_item in self.filter) ordered_filter_options = sorted(filter_options) - params.append('filter={}'.format(','.join(ordered_filter_options))) - - return "{0}?{1}".format(url, '&'.join(params)) + params['filter'] = ','.join(ordered_filter_options) + return params class _FilterOptionsBase(RequestOptionsBase): @@ -90,7 +100,7 @@ class _FilterOptionsBase(RequestOptionsBase): def __init__(self): self.view_filters = [] - def apply_query_params(self, url): + def get_query_params(self): raise NotImplementedError() def vf(self, name, value): @@ -99,7 +109,7 @@ def vf(self, name, value): def _append_view_filters(self, params): for name, value in self.view_filters: - params.append('vf_{}={}'.format(name, value)) + params['vf_' + name] = value class CSVRequestOptions(_FilterOptionsBase): @@ -116,13 +126,13 @@ def max_age(self): def max_age(self, value): self._max_age = value - def apply_query_params(self, url): - params = [] + def get_query_params(self): + params = {} if self.max_age != -1: - params.append('maxAge={0}'.format(self.max_age)) + params['maxAge'] = self.max_age self._append_view_filters(params) - return "{0}?{1}".format(url, '&'.join(params)) + return params class ImageRequestOptions(_FilterOptionsBase): @@ -144,16 +154,14 @@ def max_age(self): def max_age(self, value): self._max_age = value - def apply_query_params(self, url): - params = [] + def get_query_params(self): + params = {} if self.image_resolution: - params.append('resolution={0}'.format(self.image_resolution)) + params['resolution'] = self.image_resolution if self.max_age != -1: - params.append('maxAge={0}'.format(self.max_age)) - + params['maxAge'] = self.max_age self._append_view_filters(params) - - return "{0}?{1}".format(url, '&'.join(params)) + return params class PDFRequestOptions(_FilterOptionsBase): @@ -191,17 +199,17 @@ def max_age(self): def max_age(self, value): self._max_age = value - def apply_query_params(self, url): - params = [] + def get_query_params(self): + params = {} if self.page_type: - params.append('type={0}'.format(self.page_type)) + params['type'] = self.page_type if self.orientation: - params.append('orientation={0}'.format(self.orientation)) + params['orientation'] = self.orientation if self.max_age != -1: - params.append('maxAge={0}'.format(self.max_age)) + params['maxAge'] = self.max_age self._append_view_filters(params) - return "{0}?{1}".format(url, '&'.join(params)) + return params diff --git a/test/test_request_option.py b/test/test_request_option.py index e738a8eca..58f21aa9a 100644 --- a/test/test_request_option.py +++ b/test/test_request_option.py @@ -1,5 +1,7 @@ import unittest import os +import re +import requests import requests_mock import tableauserverclient as TSC @@ -135,6 +137,45 @@ def test_multiple_filter_options(self): matching_workbooks, pagination_item = self.server.workbooks.get(req_option) self.assertEqual(3, pagination_item.total_available) + # Test req_options if url already has query params + def test_double_query_params(self): + with requests_mock.mock() as m: + m.get(requests_mock.ANY) + url = "http://test/api/2.3/sites/12345/views?queryParamExists=true" + opts = TSC.RequestOptions() + + opts.filter.add(TSC.Filter(TSC.RequestOptions.Field.Tags, + TSC.RequestOptions.Operator.In, + ['stocks', 'market'])) + + resp = self.server.workbooks._make_request(requests.get, + url, + content=None, + request_object=opts, + auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', + content_type='text/xml') + self.assertTrue(re.search('queryparamexists=true', resp.request.query)) + self.assertTrue(re.search('filter=tags%3ain%3a%5bstocks%2cmarket%5d', resp.request.query)) + + def test_vf(self): + with requests_mock.mock() as m: + m.get(requests_mock.ANY) + url = "http://test/api/2.3/sites/123/views/456/data" + opts = TSC.PDFRequestOptions() + opts.vf("name1#", "value1") + opts.vf("name2$", "value2") + opts.page_type = TSC.PDFRequestOptions.PageType.Tabloid + + resp = self.server.workbooks._make_request(requests.get, + url, + content=None, + request_object=opts, + auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', + content_type='text/xml') + self.assertTrue(re.search('vf_name1%23=value1', resp.request.query)) + self.assertTrue(re.search('vf_name2%24=value2', resp.request.query)) + self.assertTrue(re.search('type=tabloid', resp.request.query)) + def test_multiple_filter_options_shorthand(self): with open(FILTER_MULTIPLE, 'rb') as f: response_xml = f.read().decode('utf-8') diff --git a/test/test_requests.py b/test/test_requests.py index d064e080e..c21853dbb 100644 --- a/test/test_requests.py +++ b/test/test_requests.py @@ -1,5 +1,5 @@ import unittest - +import re import requests import requests_mock @@ -22,7 +22,7 @@ def test_make_get_request(self): with requests_mock.mock() as m: m.get(requests_mock.ANY) url = "http://test/api/2.3/sites/dad65087-b08b-4603-af4e-2887b8aafc67/workbooks" - opts = TSC.RequestOptions(pagesize=13, pagenumber=13) + opts = TSC.RequestOptions(pagesize=13, pagenumber=15) resp = self.server.workbooks._make_request(requests.get, url, content=None, @@ -30,9 +30,8 @@ def test_make_get_request(self): auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', content_type='text/xml') - self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13') - self.assertEqual(resp.request.headers['x-tableau-auth'], 'j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM') - self.assertEqual(resp.request.headers['content-type'], 'text/xml') + self.assertTrue(re.search('pagesize=13', resp.request.query)) + self.assertTrue(re.search('pagenumber=15', resp.request.query)) def test_make_post_request(self): with requests_mock.mock() as m: diff --git a/test/test_sort.py b/test/test_sort.py index 40936d835..106153cf6 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -1,5 +1,5 @@ import unittest -import os +import re import requests import requests_mock import tableauserverclient as TSC @@ -31,7 +31,9 @@ def test_filter_equals(self): auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', content_type='text/xml') - self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&filter=name:eq:superstore') + self.assertTrue(re.search('pagenumber=13', resp.request.query)) + self.assertTrue(re.search('pagesize=13', resp.request.query)) + self.assertTrue(re.search('filter=name%3aeq%3asuperstore', resp.request.query)) def test_filter_equals_list(self): with self.assertRaises(ValueError) as cm: @@ -57,7 +59,9 @@ def test_filter_in(self): request_object=opts, auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', content_type='text/xml') - self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&filter=tags:in:%5bstocks,market%5d') + self.assertTrue(re.search('pagenumber=13', resp.request.query)) + self.assertTrue(re.search('pagesize=13', resp.request.query)) + self.assertTrue(re.search('filter=tags%3ain%3a%5bstocks%2cmarket%5d', resp.request.query)) def test_sort_asc(self): with requests_mock.mock() as m: @@ -74,7 +78,9 @@ def test_sort_asc(self): auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', content_type='text/xml') - self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&sort=name:asc') + self.assertTrue(re.search('pagenumber=13', resp.request.query)) + self.assertTrue(re.search('pagesize=13', resp.request.query)) + self.assertTrue(re.search('sort=name%3aasc', resp.request.query)) def test_filter_combo(self): with requests_mock.mock() as m: @@ -97,9 +103,14 @@ def test_filter_combo(self): auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM', content_type='text/xml') - expected = 'pagenumber=13&pagesize=13&filter=lastlogin:gte:2017-01-15t00:00:00:00z,siterole:eq:publisher' + expected = 'pagenumber=13&pagesize=13&filter=lastlogin%3agte%3a' \ + '2017-01-15t00%3a00%3a00%3a00z%2csiterole%3aeq%3apublisher' - self.assertEqual(resp.request.query, expected) + self.assertTrue(re.search('pagenumber=13', resp.request.query)) + self.assertTrue(re.search('pagesize=13', resp.request.query)) + self.assertTrue(re.search( + 'filter=lastlogin%3agte%3a2017-01-15t00%3a00%3a00%3a00z%2csiterole%3aeq%3apublisher', + resp.request.query)) if __name__ == '__main__':