10000 Fix filename behavior and refactor (#517) · tableau/server-client-python@82de656 · GitHub
[go: up one dir, main page]
Skip to content

Commit 82de656

Browse files
t8y8t8y8
authored
Fix filename behavior and refactor (#517)
Fixing a subtle file name bug and adding some new tests to cover filesystem helpers
1 parent ba2796f commit 82de656

File tree

6 files changed

+76
-25
lines changed

6 files changed

+76
-25
lines changed

‎setup.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
],
3131
tests_require=[
3232
'requests-mock>=1.0,<2.0',
33-
'pytest'
33+
'pytest',
34+
'mock'
3435
]
3536
)

‎tableauserverclient/filesys_helpers.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
1+
import os
12
ALLOWED_SPECIAL = (' ', '.', '_', '-')
23

34

45
def to_filename(string_to_sanitize):
56
sanitized = (c for c in string_to_sanitize if c.isalnum() or c in ALLOWED_SPECIAL)
67
return "".join(sanitized)
8+
9+
10+
def make_download_path(filepath, filename):
11+
download_path = None
12+
13+
if filepath is None:
14+
download_path = filename
15+
16+
elif os.path.isdir(filepath):
17+
download_path = os.path.join(filepath, filename)
18+
19+
else:
20+
download_path = filepath + os.path.splitext(filename)[1]
21+
22+
return download_path

‎tableauserverclient/server/endpoint/datasources_endpoint.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from .fileuploads_endpoint import Fileuploads
77
from .resource_tagger import _ResourceTagger
88
from .. import RequestFactory, DatasourceItem, PaginationItem, ConnectionItem
9-
from ...filesys_helpers import to_filename
9+
from ...filesys_helpers import to_filename, make_download_path
1010
from ...models.tag_item import TagItem
1111
from ...models.job_item import JobItem
1212
import os
@@ -104,17 +104,15 @@ def download(self, datasource_id, filepath=None, include_extract=True, no_extrac
104104
with closing(self.get_request(url, parameters={'stream': True})) as server_response:
105105
_, params = cgi.parse_header(server_response.headers['Content-Disposition'])
106106
filename = to_filename(os.path.basename(params['filename']))
107-
if filepath is None:
108-
filepath = filename
109-
elif os.path.isdir(filepath):
110-
filepath = os.path.join(filepath, filename)
111107

112-
with open(filepath, 'wb') as f:
108+
download_path = make_download_path(filepath, filename)
109+
110+
with open(download_path, 'wb') as f:
113111
for chunk in server_response.iter_content(1024): # 1KB
114112
f.write(chunk)
115113

116-
logger.info('Downloaded datasource to {0} (ID: {1})'.format(filepath, datasource_id))
117-
return os.path.abspath(filepath)
114+
logger.info('Downloaded datasource to {0} (ID: {1})'.format(download_path, datasource_id))
115+
return os.path.abspath(download_path)
118116

119117
# Update datasource
120118
@api(version="2.0")

‎tableauserverclient/server/endpoint/flows_endpoint.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from .fileuploads_endpoint import Fileuploads
77
from .resource_tagger import _ResourceTagger
88
from .. import RequestFactory, FlowItem, PaginationItem, ConnectionItem
9-
from ...filesys_helpers import to_filename
9+
from ...filesys_helpers import to_filename, make_download_path
1010
from ...models.tag_item import TagItem
1111
from ...models.job_item import JobItem
1212
import os
@@ -94,17 +94,15 @@ def download(self, flow_id, filepath=None):
9494
with closing(self.get_request(url, parameters={'stream': True})) as server_response:
9595
_, params = cgi.parse_header(server_response.headers['Content-Disposition'])
9696
filename = to_filename(os.path.basename(params['filename']))
97-
if filepath is None:
98-
filepath = filename
99-
elif os.path.isdir(filepath):
100-
filepath = os.path.join(filepath, filename)
10197

102-
with open(filepath, 'wb') as f:
98+
download_path = make_download_path(filepath, filename)
99+
100+
with open(download_path, 'wb') as f:
103101
for chunk in server_response.iter_content(1024): # 1KB
104102
f.write(chunk)
105103

106-
logger.info('Downloaded flow to {0} (ID: {1})'.format(filepath, flow_id))
107-
return os.path.abspath(filepath)
104+
logger.info('Downloaded flow to {0} (ID: {1})'.format(download_path, flow_id))
105+
return os.path.abspath(download_path)
108106

109107
# Update flow
110108
@api(version="3.3")

‎tableauserverclient/server/endpoint/workbooks_endpoint.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from .. import RequestFactory, WorkbookItem, ConnectionItem, ViewItem, PaginationItem
99
from ...models.tag_item import TagItem
1010
from ...models.job_item import JobItem
11-
from ...filesys_helpers import to_filename
11+
from ...filesys_helpers import to_filename, make_download_path
1212

1313
import os
1414
import logging
@@ -129,16 +129,14 @@ def download(self, workbook_id, filepath=None, include_extract=True, no_extract=
129129
with closing(self.get_request(url, parameters={"stream": True})) as server_response:
130130
_, params = cgi.parse_header(server_response.headers['Content-Disposition'])
131131
filename = to_filename(os.path.basename(params['filename']))
132-
if filepath is None:
133-
filepath = filename
134-
elif os.path.isdir(filepath):
135-
filepath = os.path.join(filepath, filename)
136132

137-
with open(filepath, 'wb') as f:
133+
download_path = make_download_path(filepath, filename)
134+
135+
with open(download_path, 'wb') as f:
138136
for chunk in server_response.iter_content(1024): # 1KB
139137
f.write(chunk)
140-
logger.info('Downloaded workbook to {0} (ID: {1})'.format(filepath, workbook_id))
141-
return os.path.abspath(filepath)
138+
logger.info('Downloaded workbook to {0} (ID: {1})'.format(download_path, workbook_id))
139+
return os.path.abspath(download_path)
142140

143141
# Get all views of workbook
144142
@api(version="2.0")

‎test/test_regression_tests.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
import unittest
2+
3+
try:
4+
from unittest import mock
5+
except ImportError:
6+
import mock
7+
28
import tableauserverclient.server.request_factory as factory
39
from tableauserverclient.server.endpoint import Endpoint
10+
from tableauserverclient.filesys_helpers import to_filename, make_download_path
411

512

613
class BugFix257(unittest.TestCase):
@@ -21,3 +28,36 @@ class FakeResponse(object):
2128
server_response = FakeResponse()
2229

2330
self.assertEqual(Endpoint._safe_to_log(server_response), '[Truncated File Contents]')
31+
32+
33+
class FileSysHelpers(unittest.TestCase):
34+
def test_to_filename(self):
35+
invalid = [
36+
"23brhafbjrjhkbbea.txt",
37+
'a_b_C.txt',
38+
'windows space.txt',
39+
'abc#def.txt',
40+
't@bL3A()',
41+
]
42+
43+
valid = [
44+
"23brhafbjrjhkbbea.txt",
45+
'a_b_C.txt',
46+
'windows space.txt',
47+
'abcdef.txt',
48+
'tbL3A',
49+
]
50+
51+
self.assertTrue(all([(to_filename(i) == v) for i, v in zip(invalid, valid)]))
52+
53+
def test_make_download_path(self):
54+
no_file_path = (None, 'file.ext')
55+
has_file_path_folder = ('/root/folder/', 'file.ext')
56+
has_file_path_file = ('out', 'file.ext')
57+
58+
self.assertEquals('file.ext', make_download_path(*no_file_path))
59+
self.assertEquals('out.ext', make_download_path(*has_file_path_file))
60+
61+
with mock.patch('os.path.isdir') as mocked_isdir:
62+
mocked_isdir.return_value = True
63+
self.assertEquals('/root/folder/file.ext', make_download_path(*has_file_path_folder))

0 commit comments

Comments
 (0)
0