8000 Removes manual query param generation (#686) · tableau/server-client-python@69ca6c7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 69ca6c7

Browse files
author
Chris Shin
authored
Removes manual query param generation (#686)
* Removes manual query param generation * Fixes v3.5 test issue by using regex matching * Moving all query param verifications to use regex search * Adds back removed method with deprecation warning
1 parent 1f4e27f commit 69ca6c7

File tree

5 files changed

+106
-47
lines changed

5 files changed

+106
-47
lines changed

‎tableauserverclient/server/endpoint/endpoint.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ def _safe_to_log(server_response):
4141

4242
def _make_request(self, method, url, content=None, request_object=None,
4343
auth_token=None, content_type=None, parameters=None):
44-
if request_object is not None:
45-
url = request_object.apply_query_params(url)
4644
parameters = parameters or {}
45+
if request_object is not None:
46+
parameters["params"] = request_object.get_query_params()
4747
parameters.update(self.parent_srv.http_options)
4848
parameters['headers'] = Endpoint._make_common_headers(auth_token, content_type)
4949

‎tableauserverclient/server/request_options.py

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@
33

44
class RequestOptionsBase(object):
55
def apply_query_params(self, url):
6+
import warnings
7+
warnings.simplefilter('always', DeprecationWarning)
8+
warnings.warn('apply_query_params is deprecated, please use get_query_params instead.', DeprecationWarning)
9+
try:
10+
params = self.get_query_params()
11+
params_list = ["{}={}".format(k, v) for (k, v) in params.items()]
12+
13+
if '?' in url:
14+
url, existing_params = url.split('?')
15+
params_list.append(existing_params)
16+
17+
return "{0}?{1}".format(url, '&'.join(params_list))
18+
except NotImplementedError:
19+
raise
20+
21+
def get_query_params(self):
622
raise NotImplementedError()
723

824

@@ -61,27 +77,21 @@ def page_number(self, page_number):
6177
self.pagenumber = page_number
6278
return self
6379

64-
def apply_query_params(self, url):
65-
params = []
66-
67-
if '?' in url:
68-
url, existing_params = url.split('?')
69-
params.append(existing_params)
70-
71-
if self.page_number:
72-
params.append('pageNumber={0}'.format(self.pagenumber))
73-
if self.page_size:
74-
params.append('pageSize={0}'.format(self.pagesize))
80+
def get_query_params(self):
81+
params = {}
82+
if self.pagenumber:
83+
params['pageNumber'] = self.pagenumber
84+
if self.pagesize:
85+
params['pageSize'] = self.pagesize
7586
if len(self.sort) > 0:
7687
sort_options = (str(sort_item) for sort_item in self.sort)
7788
ordered_sort_options = sorted(sort_options)
78-
params.append('sort={}'.format(','.join(ordered_sort_options)))
89+
params['sort'] = ','.join(ordered_sort_options)
7990
if len(self.filter) > 0:
8091
filter_options = (str(filter_item) for filter_item in self.filter)
8192
ordered_filter_options = sorted(filter_options)
82-
params.append('filter={}'.format(','.join(ordered_filter_options)))
83-
84-
return "{0}?{1}".format(url, '&'.join(params))
93+
params['filter'] = ','.join(ordered_filter_options)
94+
return params
8595

8696

8797
class _FilterOptionsBase(RequestOptionsBase):
@@ -90,7 +100,7 @@ class _FilterOptionsBase(RequestOptionsBase):
90100
def __init__(self):
91101
self.view_filters = []
92102

93-
def apply_query_params(self, url):
103+
def get_query_params(self):
94104
raise NotImplementedError()
95105

96106
def vf(self, name, value):
@@ -99,7 +109,7 @@ def vf(self, name, value):
99109

100110
def _append_view_filters(self, params):
101111
for name, value in self.view_filters:
102-
params.append('vf_{}={}'.format(name, value))
112+
params['vf_' + name] = value
103113

104114

105115
class CSVRequestOptions(_FilterOptionsBase):
@@ -116,13 +126,13 @@ def max_age(self):
116126
def max_age(self, value):
117127
self._max_age = value
118128

119-
def apply_query_params(self, url):
120-
params = []
129+
def get_query_params(self):
130+
params = {}
121131
if self.max_age != -1:
122-
params.append('maxAge={0}'.format(self.max_age))
132+
params['maxAge'] = self.max_age
123133

124134
self._append_view_filters(params)
125-
return "{0}?{1}".format(url, '&'.join(params))
135+
return params
126136

127137

128138
class ImageRequestOptions(_FilterOptionsBase):
@@ -144,16 +154,14 @@ def max_age(self):
144154
def max_age(self, value):
145155
self._max_age = value
146156

147-
def apply_query_params(self, url):
148-
params = []
157+
def get_query_params(self):
158+
params = {}
149159
if self.image_resolution:
150-
params.append('resolution={0}'.format(self.image_resolution))
160+
params['resolution'] = self.image_resolution
151161
if self.max_age != -1:
152-
params.append('maxAge={0}'.format(self.max_age))
153-
162+
params['maxAge'] = self.max_age
154163
self._append_view_filters(params)
155-
156-
return "{0}?{1}".format(url, '&'.join(params))
164+
return params
157165

158166

159167
class PDFRequestOptions(_FilterOptionsBase):
@@ -191,17 +199,17 @@ def max_age(self):
191199
def max_age(self, value):
192200
self._max_age = value
193201

194-
def apply_query_params(self, url):
195-
params = []
202+
def get_query_params(self):
203+
params = {}
196204
if self.page_type:
197-
params.append('type={0}'.format(self.page_type))
205+
params['type'] = self.page_type
198206

199207
if self.orientation:
200-
params.append('orientation={0}'.format(self.orientation))
208+
params['orientation'] = self.orientation
201209

202210
if self.max_age != -1:
203-
params.append('maxAge={0}'.format(self.max_age))
211+
params['maxAge'] = self.max_age
204212

205213
self._append_view_filters(params)
206214

207-
return "{0}?{1}".format(url, '&'.join(params))
215+
return params

‎test/test_request_option.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import unittest
22
import os
3+
import re
4+
import requests
35
import requests_mock
46
import tableauserverclient as TSC
57

@@ -135,6 +137,45 @@ def test_multiple_filter_options(self):
135137
matching_workbooks, pagination_item = self.server.workbooks.get(req_option)
136138
self.assertEqual(3, pagination_item.total_available)
137139

140+
# Test req_options if url already has query params
141+
def test_double_query_params(self):
142+
with requests_mock.mock() as m:
143+
m.get(requests_mock.ANY)
144+
url = "http://test/api/2.3/sites/12345/views?queryParamExists=true"
145+
opts = TSC.RequestOptions()
146+
147+
opts.filter.add(TSC.Filter(TSC.RequestOptions.Field.Tags,
148+
TSC.RequestOptions.Operator.In,
149+
['stocks', 'market']))
150+
151+
resp = self.server.workbooks._make_request(requests.get,
152+
url,
153+
content=None,
154+
request_object=opts,
155+
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
156+
content_type='text/xml')
157+
self.assertTrue(re.search('queryparamexists=true', resp.request.query))
158+
self.assertTrue(re.search('filter=tags%3ain%3a%5bstocks%2cmarket%5d', resp.request.query))
159+
160+
def test_vf(self):
161+
with requests_mock.mock() as m:
162+
m.get(requests_mock.ANY)
163+
url = "http://test/api/2.3/sites/123/views/456/data"
164+
opts = TSC.PDFRequestOptions()
165+
opts.vf("name1#", "value1")
166+
opts.vf("name2$", "value2")
167+
opts.page_type = TSC.PDFRequestOptions.PageType.Tabloid
168+
169+
resp = self.server.workbooks._make_request(requests.get,
170+
url,
171+
content=None,
172+
request_object=opts,
173+
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
174+
content_type='text/xml')
175+
self.assertTrue(re.search('vf_name1%23=value1', resp.request.query))
176+
self.assertTrue(re.search('vf_name2%24=value2', resp.request.query))
177+
self.assertTrue(re.search('type=tabloid', resp.request.query))
178+
138179
def test_multiple_filter_options_shorthand(self):
139180
with open(FILTER_MULTIPLE, 'rb') as f:
140181
response_xml = f.read().decode('utf-8')

‎test/test_requests.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import unittest
2-
2+
import re
33
import requests
44
import requests_mock
55

@@ -22,17 +22,16 @@ def test_make_get_request(self):
2222
with requests_mock.mock() as m:
2323
m.get(requests_mock.ANY)
2424
url = "http://test/api/2.3/sites/dad65087-b08b-4603-af4e-2887b8aafc67/workbooks"
25-
opts = TSC.RequestOptions(pagesize=13, pagenumber=13)
25+
opts = TSC.RequestOptions(pagesize=13, pagenumber=15)
2626
resp = self.server.workbooks._make_request(requests.get,
2727
url,
2828
content=None,
2929
request_object=opts,
3030
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
3131
content_type='text/xml')
3232

33-
self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13')
34-
self.assertEqual(resp.request.headers['x-tableau-auth'], 'j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM')
35-
self.assertEqual(resp.request.headers['content-type'], 'text/xml')
33+
self.assertTrue(re.search('pagesize=13', resp.request.query))
34+
self.assertTrue(re.search('pagenumber=15', resp.request.query))
3635

3736
def test_make_post_request(self):
3837
with requests_mock.mock() as m:

‎test/test_sort.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import unittest
2-
import os
2+
import re
33
import requests
44
import requests_mock
55
import tableauserverclient as TSC
@@ -31,7 +31,9 @@ def test_filter_equals(self):
3131
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
3232
content_type='text/xml')
3333

34-
self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&filter=name:eq:superstore')
34+
self.assertTrue(re.search('pagenumber=13', resp.request.query))
35+
self.assertTrue(re.search('pagesize=13', resp.request.query))
36+
self.assertTrue(re.search('filter=name%3aeq%3asuperstore', resp.request.query))
3537

3638
def test_filter_equals_list(self):
3739
with self.assertRaises(ValueError) as cm:
@@ -57,7 +59,9 @@ def test_filter_in(self):
5759
request_object=opts,
5860
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
5961
content_type='text/xml')
60-
self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&filter=tags:in:%5bstocks,market%5d')
62+
self.assertTrue(re.search('pagenumber=13', resp.request.query))
63+
self.assertTrue(re.search('pagesize=13', resp.request.query))
64+
self.assertTrue(re.search('filter=tags%3ain%3a%5bstocks%2cmarket%5d', resp.request.query))
6165

6266
def test_sort_asc(self):
6367
with requests_mock.mock() as m:
@@ -74,7 +78,9 @@ def test_sort_asc(self):
7478
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
7579
content_type='text/xml')
7680

77-
self.assertEqual(resp.request.query, 'pagenumber=13&pagesize=13&sort=name:asc')
81+
self.assertTrue(re.search('pagenumber=13', resp.request.query))
82+
self.assertTrue(re.search('pagesize=13', resp.request.query))
83+
self.assertTrue(re.search('sort=name%3aasc', resp.request.query))
7884

7985
def test_filter_combo(self):
8086
with requests_mock.mock() as m:
@@ -97,9 +103,14 @@ def test_filter_combo(self):
97103
auth_token='j80k54ll2lfMZ0tv97mlPvvSCRyD0DOM',
98104
content_type='text/xml')
99105

100-
expected = 'pagenumber=13&pagesize=13&filter=lastlogin:gte:2017-01-15t00:00:00:00z,siterole:eq:publisher'
106+
expected = 'pagenumber=13&pagesize=13&filter=lastlogin%3agte%3a' \
107+
'2017-01-15t00%3a00%3a00%3a00z%2csiterole%3aeq%3apublisher'
101108

102-
self.assertEqual(resp.request.query, expected)
109+
self.assertTrue(re.search('pagenumber=13', resp.request.query))
110+
self.assertTrue(re.search('pagesize=13', resp.request.query))
111+
self.assertTrue(re.search(
112+
'filter=lastlogin%3agte%3a2017-01-15t00%3a00%3a00%3a00z%2csiterole%3aeq%3apublisher',
113+
resp.request.query))
103114

104115

105116
if __name__ == '__main__':

0 commit comments

Comments
 (0)
0