Removes manual query param generation

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)
 

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

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')

test/test_requests.py

@@ -1,5 +1,5 @@
 import unittest
-
+import re
 import requests
 import requests_mock
 
@@ -22,17 +22,16 @@ 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,
                                                        request_object=opts,
                                                        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:

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__':