Browse Source

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 8417c169171b3cfbbe195522287700c4d7050d3f.

* 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
Daniel Griesser 8 years ago
parent
commit
c516cc8ad4

+ 1 - 0
CHANGES

@@ -15,6 +15,7 @@ Version 8.12 (Unreleased)
 - Added ``firstRelease`` to search (uses ``first_release``).
 - Fixed usage (and propagation) of ``Group.first_release``.
 - The + and - datetime search helpers now work with ranges (e.g. ``<=``).
+- Added the ability to download artifacts from releases.
 
 SDKs
 ~~~~

+ 18 - 0
src/sentry/api/endpoints/release_file_details.py

@@ -9,6 +9,7 @@ from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.serializers import serialize
 from sentry.models import Release, ReleaseFile
 from sentry.utils.apidocs import scenario, attach_scenarios
+from django.http import CompatibleStreamingHttpResponse
 
 
 @scenario('RetrieveReleaseFile')
@@ -70,6 +71,17 @@ class ReleaseFileDetailsEndpoint(ProjectEndpoint):
     doc_section = DocSection.RELEASES
     permission_classes = (ProjectReleasePermission,)
 
+    def download(self, releasefile):
+        file = releasefile.file
+        fp = file.getfile()
+        response = CompatibleStreamingHttpResponse(
+            iter(lambda: fp.read(4096), b''),
+            content_type=file.headers.get('content-type', 'application/octet-stream'),
+        )
+        response['Content-Length'] = file.size
+        response['Content-Disposition'] = "attachment; filename=%s" % releasefile.name
+        return response
+
     @attach_scenarios([retrieve_file_scenario])
     def get(self, request, project, version, file_id):
         """
@@ -104,6 +116,12 @@ class ReleaseFileDetailsEndpoint(ProjectEndpoint):
         except ReleaseFile.DoesNotExist:
             raise ResourceDoesNotExist
 
+        download_requested = request.GET.get('download') is not None
+        if download_requested and (
+           request.access.has_scope('project:write')):
+            return self.download(releasefile)
+        elif download_requested:
+            return Response(status=403)
         return Response(serialize(releasefile, request.user))
 
     @attach_scenarios([update_file_scenario])

+ 29 - 5
src/sentry/static/sentry/app/views/releaseArtifacts.jsx

@@ -1,6 +1,8 @@
 import React from 'react';
 
 import ApiMixin from '../mixins/apiMixin';
+import OrganizationState from '../mixins/organizationState';
+import TooltipMixin from '../mixins/tooltip';
 import FileSize from '../components/fileSize';
 import LoadingError from '../components/loadingError';
 import LoadingIndicator from '../components/loadingIndicator';
@@ -15,7 +17,14 @@ const ReleaseArtifacts = React.createClass({
     release: React.PropTypes.object
   },
 
-  mixins: [ApiMixin],
+  mixins: [
+    ApiMixin, 
+    OrganizationState, 
+    TooltipMixin({
+      selector: '.tip',
+      trigger: 'hover'
+    })
+  ],
 
   getInitialState() {
     return {
@@ -57,6 +66,7 @@ const ReleaseArtifacts = React.createClass({
           fileList: data,
           pageLinks: jqXHR.getResponseHeader('Link')
         });
+        this.attachTooltips();
       },
       error: () => {
         this.setState({
@@ -109,23 +119,37 @@ const ReleaseArtifacts = React.createClass({
         </div>
       );
 
+    let access = this.getAccess();
+    
     // TODO(dcramer): files should allow you to download them
     return (
       <div>
         <div className="release-group-header">
           <div className="row">
-            <div className="col-sm-9 col-xs-8">{'Name'}</div>
+            <div className="col-sm-8 col-xs-7">{'Name'}</div>
             <div className="col-sm-2 col-xs-2 align-right">{'Size'}</div>
-            <div className="col-sm-1 col-xs-2 align-right"></div>
+            <div className="col-sm-2 col-xs-3 align-right"></div>
           </div>
         </div>
         <div className="release-list">
         {this.state.fileList.map((file) => {
           return (
             <div className="release release-artifact row" key={file.id}>
-              <div className="col-sm-9 col-xs-8" style={{wordWrap: 'break-word'}}><strong>{file.name || '(empty)'}</strong></div>
+              <div className="col-sm-8 col-xs-7" style={{wordWrap: 'break-word'}}><strong>{file.name || '(empty)'}</strong></div>
               <div className="col-sm-2 col-xs-2 align-right"><FileSize bytes={file.size} /></div>
-              <div className="col-sm-1 col-xs-2 align-right">
+              <div className="col-sm-2 col-xs-3 align-right actions">
+                {access.has('project:write') ?
+                  <a
+                      href={this.api.baseUrl + this.getFilesEndpoint() + `${file.id}/?download=1`}
+                      className="btn btn-sm btn-default">
+                      <span className="icon icon-open" />
+                  </a>
+                  :
+                  <div
+                    className="btn btn-sm btn-default disabled tip" title={t('You do not have the required permission to download this artifact.')}>
+                    <span className="icon icon-open" />
+                  </div>
+                }
                 <LinkWithConfirmation
                   className="btn btn-sm btn-default"
                   title={t('Delete artifact')}

+ 6 - 0
src/sentry/static/sentry/less/releases.less

@@ -56,6 +56,12 @@
     padding: 15px 0;
     margin-left: 0;
     margin-right: 0;
+    .btn-sm {
+      margin-left: 5px;
+    }
+    .actions {
+      white-space: nowrap;
+    }
   }
 
   .release-meta {

+ 55 - 2
tests/js/spec/views/releaseArtifacts.spec.jsx

@@ -13,8 +13,25 @@ describe('ReleaseArtifacts', function() {
 
     this.wrapper = shallow(<ReleaseArtifacts
       location={{query: {cursor: '0:0:100'}}}
-      params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>
-    );
+      params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>, {
+      context: {
+        group: {id: '1337'},
+        project: {id: 'foo'},
+        team: {id: '1'},
+        organization: {id:'bar'}
+      }
+    });
+
+    this.wrapperWithPermission = shallow(<ReleaseArtifacts
+      location={{query: {cursor: '0:0:100'}}}
+      params={{orgId: '123', projectId: '456', version: 'abcdef'}}/>, {
+      context: {
+        group: {id: '1337'},
+        project: {id: 'foo'},
+        team: {id: '1'},
+        organization: {id:'bar', access: ['project:write']}
+      }
+    });
   });
 
   afterEach(function() {
@@ -39,6 +56,42 @@ describe('ReleaseArtifacts', function() {
 
       expect(wrapper.find('.release-artifact')).to.have.length(2);
     });
+
+    it('should have no permission to download', function () {
+      let wrapper = this.wrapper;
+      wrapper.setState({
+        loading: false,
+        fileList: [{
+          id: '1',
+          name: 'foo.js',
+          size: 150000
+        }, {
+          id: '2',
+          name: 'bar.js',
+          size: 30000
+        }]
+      });
+
+      expect(wrapper.find('div.btn > .icon-open')).to.have.length(2);
+    });
+
+    it('should have permission to download', function () {
+      let wrapper = this.wrapperWithPermission;
+      wrapper.setState({
+        loading: false,
+        fileList: [{
+          id: '1',
+          name: 'foo.js',
+          size: 150000
+        }, {
+          id: '2',
+          name: 'bar.js',
+          size: 30000
+        }]
+      });
+
+      expect(wrapper.find('a.btn > .icon-open')).to.have.length(2);
+    });
   });
 
   describe('handleRemove()', function () {

+ 46 - 0
tests/sentry/api/endpoints/test_release_file_details.py

@@ -44,6 +44,52 @@ class ReleaseFileDetailsTest(APITestCase):
         assert response.status_code == 200, response.content
         assert response.data['id'] == six.text_type(releasefile.id)
 
+    def test_file_download(self):
+        self.login_as(user=self.user)
+
+        project = self.create_project(name='foo')
+
+        release = Release.objects.create(
+            project=project,
+            organization_id=project.organization_id,
+            version='1',
+        )
+        release.add_project(project)
+
+        from six import BytesIO
+        f = File.objects.create(
+            name='application.js',
+            type='release.file',
+        )
+        f.putfile(BytesIO('File contents here'))
+
+        releasefile = ReleaseFile.objects.create(
+            organization_id=project.organization_id,
+            project=project,
+            release=release,
+            file=f,
+            name='http://example.com/application.js'
+        )
+
+        url = reverse('sentry-api-0-release-file-details', kwargs={
+            'organization_slug': project.organization.slug,
+            'project_slug': project.slug,
+            'version': release.version,
+            'file_id': releasefile.id,
+        })
+
+        response = self.client.get(url + '?download=1')
+        assert response.status_code == 200, response.content
+        assert response.get('Content-Disposition') == "attachment; filename=http://example.com/application.js"
+        assert response.get('Content-Length') == six.text_type(f.size)
+        assert response.get('Content-Type') == 'application/octet-stream'
+        assert 'File contents here' == BytesIO(b"".join(response.streaming_content)).getvalue()
+
+        user_no_permission = self.create_user('baz@localhost', username='baz')
+        self.login_as(user=user_no_permission)
+        response = self.client.get(url + '?download=1')
+        assert response.status_code == 403, response.content
+
 
 class ReleaseFileUpdateTest(APITestCase):
     def test_simple(self):