Skip to content

Commit c516cc8

Browse files
authored
Add download to release files (#4704)
* Add download to release files * Add permission check for releasefile download * Remove console.log statement * Reformat if statement * Remove project:releases permission, Add streamed download, Add tooltip * Add translations * Add context to tests * Revert "Add translations" This reverts commit 8417c16. * Return 403 response if download is not allowed * Add simple frontend tests for release artifact download * Add integration test for release artifact download * Fix integration test for artifact download * Added entry to CHANGES * Add additional asserts for artifact download * Add content-type assert * Fix content-type assert
1 parent 3bb0ddb commit c516cc8

File tree

6 files changed

+155
-7
lines changed

6 files changed

+155
-7
lines changed

CHANGES

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Version 8.12 (Unreleased)
1515
- Added ``firstRelease`` to search (uses ``first_release``).
1616
- Fixed usage (and propagation) of ``Group.first_release``.
1717
- The + and - datetime search helpers now work with ranges (e.g. ``<=``).
18+
- Added the ability to download artifacts from releases.
1819

1920
SDKs
2021
~~~~

src/sentry/api/endpoints/release_file_details.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from sentry.api.serializers import serialize
1010
from sentry.models import Release, ReleaseFile
1111
from sentry.utils.apidocs import scenario, attach_scenarios
12+
from django.http import CompatibleStreamingHttpResponse
1213

1314

1415
@scenario('RetrieveReleaseFile')
@@ -70,6 +71,17 @@ class ReleaseFileDetailsEndpoint(ProjectEndpoint):
7071
doc_section = DocSection.RELEASES
7172
permission_classes = (ProjectReleasePermission,)
7273

74+
def download(self, releasefile):
75+
file = releasefile.file
76+
fp = file.getfile()
77+
response = CompatibleStreamingHttpResponse(
78+
iter(lambda: fp.read(4096), b''),
79+
content_type=file.headers.get('content-type', 'application/octet-stream'),
80+
)
81+
response['Content-Length'] = file.size
82+
response['Content-Disposition'] = "attachment; filename=%s" % releasefile.name
83+
return response
84+
7385
@attach_scenarios([retrieve_file_scenario])
7486
def get(self, request, project, version, file_id):
7587
"""
@@ -104,6 +116,12 @@ def get(self, request, project, version, file_id):
104116
except ReleaseFile.DoesNotExist:
105117
raise ResourceDoesNotExist
106118

119+
download_requested = request.GET.get('download') is not None
120+
if download_requested and (
121+
request.access.has_scope('project:write')):
122+
return self.download(releasefile)
123+
elif download_requested:
124+
return Response(status=403)
107125
return Response(serialize(releasefile, request.user))
108126

109127
@attach_scenarios([update_file_scenario])

src/sentry/static/sentry/app/views/releaseArtifacts.jsx

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import React from 'react';
22

33
import ApiMixin from '../mixins/apiMixin';
4+
import OrganizationState from '../mixins/organizationState';
5+
import TooltipMixin from '../mixins/tooltip';
46
import FileSize from '../components/fileSize';
57
import LoadingError from '../components/loadingError';
68
import LoadingIndicator from '../components/loadingIndicator';
@@ -15,7 +17,14 @@ const ReleaseArtifacts = React.createClass({
1517
release: React.PropTypes.object
1618
},
1719

18-
mixins: [ApiMixin],
20+
mixins: [
21+
ApiMixin,
22+
OrganizationState,
23+
TooltipMixin({
24+
selector: '.tip',
25+
trigger: 'hover'
26+
})
27+
],
1928

2029
getInitialState() {
2130
return {
@@ -57,6 +66,7 @@ const ReleaseArtifacts = React.createClass({
5766
fileList: data,
5867
pageLinks: jqXHR.getResponseHeader('Link')
5968
});
69+
this.attachTooltips();
6070
},
6171
error: () => {
6272
this.setState({
@@ -109,23 +119,37 @@ const ReleaseArtifacts = React.createClass({
109119
</div>
110120
);
111121

122+
let access = this.getAccess();
123+
112124
// TODO(dcramer): files should allow you to download them
113125
return (
114126
<div>
115127
<div className="release-group-header">
116128
<div className="row">
117-
<div className="col-sm-9 col-xs-8">{'Name'}</div>
129+
<div className="col-sm-8 col-xs-7">{'Name'}</div>
118130
<div className="col-sm-2 col-xs-2 align-right">{'Size'}</div>
119-
<div className="col-sm-1 col-xs-2 align-right"></div>
131+
<div className="col-sm-2 col-xs-3 align-right"></div>
120132
</div>
121133
</div>
122134
<div className="release-list">
123135
{this.state.fileList.map((file) => {
124136
return (
125137
<div className="release release-artifact row" key={file.id}>
126-
<div className="col-sm-9 col-xs-8" style={{wordWrap: 'break-word'}}><strong>{file.name || '(empty)'}</strong></div>
138+
<div className="col-sm-8 col-xs-7" style={{wordWrap: 'break-word'}}><strong>{file.name || '(empty)'}</strong></div>
127139
<div className="col-sm-2 col-xs-2 align-right"><FileSize bytes={file.size} /></div>
128-
<div className="col-sm-1 col-xs-2 align-right">
140+
<div className="col-sm-2 col-xs-3 align-right actions">
141+
{access.has('project:write') ?
142+
<a
143+
href={this.api.baseUrl + this.getFilesEndpoint() + `${file.id}/?download=1`}
144+
className="btn btn-sm btn-default">
145+
<span className="icon icon-open" />
146+
</a>
147+
:
148+
<div
149+
className="btn btn-sm btn-default disabled tip" title={t('You do not have the required permission to download this artifact.')}>
150+
<span className="icon icon-open" />
151+
</div>
152+
}
129153
<LinkWithConfirmation
130154
className="btn btn-sm btn-default"
131155
title={t('Delete artifact')}

src/sentry/static/sentry/less/releases.less

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@
5656
padding: 15px 0;
5757
margin-left: 0;
5858
margin-right: 0;
59+
.btn-sm {
60+
margin-left: 5px;
61+
}
62+
.actions {
63+
white-space: nowrap;
64+
}
5965
}
6066

6167
.release-meta {

tests/js/spec/views/releaseArtifacts.spec.jsx

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,25 @@ describe('ReleaseArtifacts', function() {
1313

1414
this.wrapper = shallow(<ReleaseArtifacts
1515
location={{query: {cursor: '0:0:100'}}}
16-
params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>
17-
);
16+
params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>, {
17+
context: {
18+
group: {id: '1337'},
19+
project: {id: 'foo'},
20+
team: {id: '1'},
21+
organization: {id:'bar'}
22+
}
23+
});
24+
25+
this.wrapperWithPermission = shallow(<ReleaseArtifacts
26+
location={{query: {cursor: '0:0:100'}}}
27+
params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>, {
28+
context: {
29+
group: {id: '1337'},
30+
project: {id: 'foo'},
31+
team: {id: '1'},
32+
organization: {id:'bar', access: ['project:write']}
33+
}
34+
});
1835
});
1936

2037
afterEach(function() {
@@ -39,6 +56,42 @@ describe('ReleaseArtifacts', function() {
3956

4057
expect(wrapper.find('.release-artifact')).to.have.length(2);
4158
});
59+
60+
it('should have no permission to download', function () {
61+
let wrapper = this.wrapper;
62+
wrapper.setState({
63+
loading: false,
64+
fileList: [{
65+
id: '1',
66+
name: 'foo.js',
67+
size: 150000
68+
}, {
69+
id: '2',
70+
name: 'bar.js',
71+
size: 30000
72+
}]
73+
});
74+
75+
expect(wrapper.find('div.btn > .icon-open')).to.have.length(2);
76+
});
77+
78+
it('should have permission to download', function () {
79+
let wrapper = this.wrapperWithPermission;
80+
wrapper.setState({
81+
loading: false,
82+
fileList: [{
83+
id: '1',
84+
name: 'foo.js',
85+
size: 150000
86+
}, {
87+
id: '2',
88+
name: 'bar.js',
89+
size: 30000
90+
}]
91+
});
92+
93+
expect(wrapper.find('a.btn > .icon-open')).to.have.length(2);
94+
});
4295
});
4396

4497
describe('handleRemove()', function () {

tests/sentry/api/endpoints/test_release_file_details.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,52 @@ def test_simple(self):
4444
assert response.status_code == 200, response.content
4545
assert response.data['id'] == six.text_type(releasefile.id)
4646

47+
def test_file_download(self):
48+
self.login_as(user=self.user)
49+
50+
project = self.create_project(name='foo')
51+
52+
release = Release.objects.create(
53+
project=project,
54+
organization_id=project.organization_id,
55+
version='1',
56+
)
57+
release.add_project(project)
58+
59+
from six import BytesIO
60+
f = File.objects.create(
61+
name='application.js',
62+
type='release.file',
63+
)
64+
f.putfile(BytesIO('File contents here'))
65+
66+
releasefile = ReleaseFile.objects.create(
67+
organization_id=project.organization_id,
68+
project=project,
69+
release=release,
70+
file=f,
71+
name='http://example.com/application.js'
72+
)
73+
74+
url = reverse('sentry-api-0-release-file-details', kwargs={
75+
'organization_slug': project.organization.slug,
76+
'project_slug': project.slug,
77+
'version': release.version,
78+
'file_id': releasefile.id,
79+
})
80+
81+
response = self.client.get(url + '?download=1')
82+
assert response.status_code == 200, response.content
83+
assert response.get('Content-Disposition') == "attachment; filename=http://example.com/application.js"
84+
assert response.get('Content-Length') == six.text_type(f.size)
85+
assert response.get('Content-Type') == 'application/octet-stream'
86+
assert 'File contents here' == BytesIO(b"".join(response.streaming_content)).getvalue()
87+
88+
user_no_permission = self.create_user('baz@localhost', username='baz')
89+
self.login_as(user=user_no_permission)
90+
response = self.client.get(url + '?download=1')
91+
assert response.status_code == 403, response.content
92+
4793

4894
class ReleaseFileUpdateTest(APITestCase):
4995
def test_simple(self):

0 commit comments

Comments
 (0)