Эх сурвалжийг харах

HUE-705 [fb] No error message when a file upload failed

Refactored file uploading for adding success/failure notifications
Removed old templates
Added more tests
Added do_as_user(), do_as_useradmin() to the HDFS API
Romain Rigaux 13 жил өмнө
parent
commit
905a41f

+ 4 - 3
apps/filebrowser/src/filebrowser/templates/listdir.mako

@@ -13,9 +13,8 @@
 ## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 ## See the License for the specific language governing permissions and
 ## limitations under the License.
+
 <%!
-import datetime
-from django.template.defaultfilters import urlencode, stringformat, filesizeformat, date, time, escape
 from desktop.views import commonheader, commonfooter
 from django.utils.translation import ugettext as _
 %>
@@ -24,6 +23,7 @@ from django.utils.translation import ugettext as _
 <%namespace name="fb_components" file="fb_components.mako" />
 
 ${commonheader(_('File Browser'), 'filebrowser')}
+
 <div class="container-fluid">
     <h1>${_('File Browser')}</h1>
     % if breadcrumbs:
@@ -33,4 +33,5 @@ ${commonheader(_('File Browser'), 'filebrowser')}
         ${dir.list_table_browser(files, path_enc, current_request_path, cwd_set)}
     </div>
 </div>
-${commonfooter()}
+
+${commonfooter(messages)}

+ 17 - 16
apps/filebrowser/src/filebrowser/templates/listdir_components.mako

@@ -39,18 +39,23 @@ from django.utils.translation import ugettext as _
     %>
     href="?pagenum=${num}&pagesize=${pagesize}${sortby_param}${descending_param}${filter_param}"
 </%def>
+
 <%def name="prevpage(page)">
   ${pageref(page.previous_page_number())}
 </%def>
+
 <%def name="nextpage(page)">
   ${pageref(page.next_page_number())}
 </%def>
+
 <%def name="toppage(page)">
   ${pageref(1)}
 </%def>
+
 <%def name="bottompage(page)">
   ${pageref(page.num_pages())}
 </%def>
+
 <%def name="pagination(localpage)">
     <div class="pagination">
         <ul class="pull-right">
@@ -70,13 +75,13 @@ from django.utils.translation import ugettext as _
 <%def name="list_table_chooser(files, path, current_request_path)">
   ${_table(files, path, current_request_path, 'chooser')}
 </%def>
+
 <%def name="list_table_browser(files, path, current_request_path, cwd_set=True)">
   ${_table(files, path, current_request_path, 'view', cwd_set)}
 </%def>
+
 <%def name="_table(files, path, current_request_path, view, cwd_set=False)">
-    <script src="/static/ext/js/fileuploader.js" type="text/javascript" charset="utf-8"></script>
     <script src="/static/ext/js/datatables-paging-0.1.js" type="text/javascript" charset="utf-8"></script>
-    <link rel="stylesheet" href="/static/ext/css/fileuploader.css" type="text/css" media="screen" title="no title" charset="utf-8" />
     <style type="text/css">
         .pull-right {
             margin: 4px;
@@ -91,7 +96,9 @@ from django.utils.translation import ugettext as _
             display: none;
         }
     </style>
-    %if len(files)>0 :
+
+    %if len(files) > 0:
+
     <table class="table table-condensed table-striped datatables">
         <thead>
             <tr>
@@ -188,7 +195,7 @@ from django.utils.translation import ugettext as _
         </div>
     %endif
 
-%if len(files)>0 :
+%if len(files) > 0:
     <!-- delete modal -->
     <div id="deleteModal" class="modal hide fade">
         <div class="modal-header">
@@ -229,19 +236,15 @@ from django.utils.translation import ugettext as _
         </form>
     </div>
 
-    <div id="changeOwnerModal" class="modal hide fade">
-    </div>
+    <div id="changeOwnerModal" class="modal hide fade"></div>
 
-    <div id="changePermissionModal" class="modal hide fade">
-    </div>
+    <div id="changePermissionModal" class="modal hide fade"></div>
 
-    <div id="moveModal" class="modal hide fade">
-    </div>
+    <div id="moveModal" class="modal hide fade"></div>
 %endif
 
 <!-- upload modal -->
 <div id="uploadModal" class="modal hide fade">
-    <form id="uploadForm" action="/filebrowser/rename?next=${current_request_path}" method="POST" enctype="multipart/form-data" class="form-stacked form-padding-fix">
     <div class="modal-header">
         <a href="#" class="close" data-dismiss="modal">&times;</a>
         <h3>${_('Uploading to:')} <span id="uploadDirName">${current_dir_path}</span></h3>
@@ -256,10 +259,9 @@ from django.utils.translation import ugettext as _
         </form>
     </div>
     <div class="modal-footer"></div>
-    </form>
 </div>
 
-<!-- create directory modal -->
+<!-- new directory modal -->
 <div id="createDirectoryModal" class="modal hide fade">
     <form id="createDirectoryForm" action="/filebrowser/mkdir?next=${current_request_path}" method="POST" enctype="multipart/form-data" class="form-inline form-padding-fix">
     <div class="modal-header">
@@ -281,8 +283,7 @@ from django.utils.translation import ugettext as _
 </div>
 
 <script type="text/javascript" charset="utf-8">
-
-    %if len(files)>0 :
+    %if len(files) > 0 :
     // ajax modal windows
     function openChownWindow(path, user, group, next){
         $.ajax({
@@ -401,7 +402,7 @@ from django.utils.translation import ugettext as _
     $(document).ready(function(){
         var qs = getQueryString();
 
-    %if len(files)>0 :
+    %if len(files) > 0:
         if (qs["sortby"] == null){
             qs["sortby"] = "name";
         }

+ 0 - 72
apps/filebrowser/src/filebrowser/templates/upload.mako

@@ -1,72 +0,0 @@
-## Licensed to Cloudera, Inc. under one
-## or more contributor license agreements.  See the NOTICE file
-## distributed with this work for additional information
-## regarding copyright ownership.  Cloudera, Inc. licenses this file
-## to you under the Apache License, Version 2.0 (the
-## "License"); you may not use this file except in compliance
-## with the License.  You may obtain a copy of the License at
-##
-##     http://www.apache.org/licenses/LICENSE-2.0
-##
-## Unless required by applicable law or agreed to in writing, software
-## distributed under the License is distributed on an "AS IS" BASIS,
-## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-## See the License for the specific language governing permissions and
-## limitations under the License.
-<%!
-from django.utils.translation import ugettext as _
-%>
-<%namespace name="edit" file="editor_components.mako" />
-
-
-<script src="/static/ext/js/fileuploader.js" type="text/javascript" charset="utf-8"></script>
-<link rel="stylesheet" href="/static/ext/css/fileuploader.css" type="text/css" media="screen" title="no title" charset="utf-8" />
-
-
-<div class="well">
-    <form action="/filebrowser/upload?next=${next|u}" method="POST" enctype="multipart/form-data" class="form-stacked">
-        <h1>${_('Upload Files')}</h1>
-        <div id="file-uploader">
-            <noscript>
-                <p>${_('Please enable JavaScript to use the file uploader.')}</p>
-                <!-- or put a simple form for upload here -->
-            </noscript>
-        </div>
-    </form>
-</div>
-
-<!--<span class="alert-message block-message info">Go back to where you were: <a href="/filebrowser/view${next}">${next}</a>.</span>-->
-
-<script>
-    function createUploader(){
-        var uploader = new qq.FileUploader({
-            element: document.getElementById('file-uploader'),
-            action: '/filebrowser/upload',
-            template: '<div class="qq-uploader">' +
-                    '<div class="qq-upload-drop-area"><span>${_('Drop files here to upload')}</span></div>' +
-                    '<div class="qq-upload-button">${_('Upload a file')}</div>' +
-                    '<ul class="qq-upload-list"></ul>' +
-                    '</div>',
-            fileTemplate: '<li>' +
-                    '<span class="qq-upload-file"></span>' +
-                    '<span class="qq-upload-spinner"></span>' +
-                    '<span class="qq-upload-size"></span>' +
-                    '<a class="qq-upload-cancel" href="#">${_('Cancel')}</a>' +
-                    '<span class="qq-upload-failed-text">${_('Failed')}</span>' +
-                    '</li>',
-            params:{
-                dest: '${next}',
-                fileFieldLabel: 'hdfs_file'
-            },
-            onComplete:function(id, fileName, responseJSON){
-                window.location = "/filebrowser/view${next}";
-            },
-            debug: true
-        });
-    }
-
-    // in your app create uploader as soon as the DOM is ready
-    // don't wait for the window to load
-    window.onload = createUploader;
-</script>
-

+ 0 - 24
apps/filebrowser/src/filebrowser/templates/upload_done.mako

@@ -1,24 +0,0 @@
-## Licensed to Cloudera, Inc. under one
-## or more contributor license agreements.  See the NOTICE file
-## distributed with this work for additional information
-## regarding copyright ownership.  Cloudera, Inc. licenses this file
-## to you under the Apache License, Version 2.0 (the
-## "License"); you may not use this file except in compliance
-## with the License.  You may obtain a copy of the License at
-##
-##     http://www.apache.org/licenses/LICENSE-2.0
-##
-## Unless required by applicable law or agreed to in writing, software
-## distributed under the License is distributed on an "AS IS" BASIS,
-## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-## See the License for the specific language governing permissions and
-## limitations under the License.
-<%!
-import datetime
-from django.template.defaultfilters import urlencode
-from django.utils.translation import ugettext as _
-%>
-<% path_enc = urlencode(path) %>
-${_('View uploaded file:')} <a href="${url('filebrowser.views.view', path=path_enc)}">${path}</a>.<br>
-${_('Go back to where you were:')} <a href="${next|urlencode}">${next}</a>.
-

+ 67 - 59
apps/filebrowser/src/filebrowser/views.py

@@ -27,7 +27,10 @@ import operator
 import posixpath
 import stat as stat_module
 import os
-import simplejson
+try:
+  import json
+except ImportError:
+  import simplejson as json
 
 from django.contrib import messages
 from django.core import urlresolvers
@@ -854,82 +857,87 @@ def upload_flash(request):
         else:
             raise Exception("Unknown error")
     except Exception, e:
-        return HttpResponse(simplejson.dumps(dict(error=unicode(e))),
+        return HttpResponse(json.dumps(dict(error=unicode(e))),
                             content_type="application/json")
 
 
 def upload(request):
     """
-    A wrapper around the actual upload view function to clean up the
-    temporary file afterwards.
+    A wrapper around the actual upload view function to clean up the temporary file afterwards.
+
+    Returns JSON.
+    e.g. {'status' 0/1, data:'message'...}
     """
-    try:
-        return _upload(request)
-    finally:
-        if request.method == 'POST':
+    response = {'status': -1, 'data': ''}
+
+    if request.method == 'POST':
+        try:
             try:
-                upload_file = request.FILES['hdfs_file']
-                upload_file.remove()
-            except KeyError:
-                pass
+                resp = _upload(request)
+                response.update(resp)
+            except Exception, ex:
+                response['data'] = str(ex)
+        finally:
+            hdfs_file = request.FILES.get('hdfs_file')
+            if hdfs_file:
+                hdfs_file.remove()
+    else:
+        response['data'] = _('A POST request is required.')
+
+    if response['status'] == 0:
+        request.info(_('%(destination)s upload succeded') % {'destination': response['path']})
+    else:
+        request.error(_('Upload failed: %(data)s') % {'data': response['data']})
+
+    return HttpResponse(json.dumps(response), mimetype="application/json")
 
 
 def _upload(request):
     """
-    Handles file uploads. The uploaded file is stored in HDFS. We just
-    need to rename it to the right path.
+    Handles file uploaded by HDFSfileUploadHandler.
+    The uploaded file is stored in HDFS. We just need to rename it to the destination path.
     """
-    if request.method == 'POST':
-        form = UploadForm(request.POST, request.FILES)
-        if not form.is_valid():
-            logger.error("Error in upload form: %s" % (form.errors,))
-        else:
-            uploaded_file = request.FILES['hdfs_file']
-            dest = form.cleaned_data["dest"]
-            if request.fs.isdir(dest):
-                assert posixpath.sep not in uploaded_file.name
-                dest = request.fs.join(dest, uploaded_file.name)
+    form = UploadForm(request.POST, request.FILES)
+
+    if form.is_valid():
+        uploaded_file = request.FILES['hdfs_file']
+        dest = form.cleaned_data['dest']
 
+        if request.fs.isdir(dest) and posixpath.sep in uploaded_file.name:
+            raise PopupException(_('Sorry, no "%(sep)s" in the filename %(name)s.' % {'sep': posixpath.sep, 'name': uploaded_file.name}))
+
+        dest = request.fs.join(dest, uploaded_file.name)
+        tmp_file = uploaded_file.get_temp_path()
+        username = request.user.username
+
+        try:
             # Temp file is created by superuser. Chown the file.
-            tmp_file = uploaded_file.get_temp_path()
-            username = request.user.username
-            try:
-                try:
-                    request.fs.setuser(request.fs.superuser)
-                    request.fs.chmod(tmp_file, 0644)
-                    request.fs.chown(tmp_file, username, username)
-                except IOError, ex:
-                    msg = _('Failed to chown uploaded file ("%(file)s") as superuser %(superuser)s.') %\
-                          {'file': tmp_file, 'superuser': request.fs.superuser}
-                    logger.exception(msg)
-                    raise PopupException(msg, detail=str(ex))
-            finally:
-                request.fs.setuser(username)
+            request.fs.do_as_superuser(request.fs.chmod, tmp_file, 0644)
+            request.fs.do_as_superuser(request.fs.chown, tmp_file, username, username)
 
             # Move the file to where it belongs
+            request.fs.rename(uploaded_file.get_temp_path(), dest)
+        except IOError, ex:
+            already_exists = False
             try:
-                request.fs.rename(uploaded_file.get_temp_path(), dest)
-            except IOError, ex:
-                raise PopupException(
-                    _('Failed to rename uploaded temporary file ("%(file)s") to "%(name)s": %(error)s') %
-                    {'file': tmp_file, 'name': dest, 'error': ex})
-
-            dest_stats = request.fs.stats(dest)
-            return render('upload_done.mako', request, {
-                # status is used by "fancy uploader"
-                'status': 1,
-                'path': dest,
-                'result': _massage_stats(request, dest_stats),
-                'next': request.GET.get("next")
-            })
+                already_exists = request.fs.exists(dest)
+            except Exception:
+              pass
+            if already_exists:
+                msg = _('Destination %(name)s already exists.' % {'name': dest})
+            else:
+                msg = _('Copy to "%(name)s failed: %(error)s') % {'name': dest, 'error': ex}
+            raise PopupException(msg)
+
+        return {
+          'status': 0,
+          'path': dest,
+          'result': _massage_stats(request, request.fs.stats(dest)),
+          'next': request.GET.get("next")
+          }
     else:
-        dest = request.GET.get("dest")
-        initial_values = {}
-        if dest:
-            initial_values["dest"] = dest
-        form = UploadForm(initial=initial_values)
-    return render('upload.mako', request,
-            {'form': form, 'next': request.REQUEST.get("dest")})
+        raise PopupException(_("Error in upload form: %s") % (form.errors,))
+
 
 
 def status(request):

+ 48 - 19
apps/filebrowser/src/filebrowser/views_test.py

@@ -18,26 +18,31 @@
 """
 Tests for filebrowser views
 """
-from django.utils.encoding import smart_str
-from nose.plugins.attrib import attr
-from hadoop import pseudo_hdfs4
-from avro import schema, datafile, io
-from desktop.lib.django_test_util import make_logged_in_client
-from desktop.lib.django_util import PopupException
-from nose.tools import assert_true, assert_false, assert_equal, assert_raises
-from lib.rwx import expand_mode
-
 try:
   import json
 except ImportError:
   import simplejson as json
 
 import logging
+import os
 import re
 import urlparse
 
+from django.utils.encoding import smart_str
+from nose.plugins.attrib import attr
+from nose.tools import assert_true, assert_false, assert_equal, assert_raises
+
+from desktop.lib.django_test_util import make_logged_in_client
+from desktop.lib.test_utils import grant_access
+from hadoop import pseudo_hdfs4
+
+from avro import schema, datafile, io
+from lib.rwx import expand_mode
+
+
 LOG = logging.getLogger(__name__)
 
+
 @attr('requires_hadoop')
 def test_mkdir_singledir():
   cluster = pseudo_hdfs4.shared_cluster()
@@ -248,7 +253,7 @@ def test_listdir_sort_and_filter():
 
     listing = c.get('/filebrowser/view' + BASE + '?sortby=name&descending=true').context['files']
     assert_equal(sorted(expect, reverse=True), [ f['name'] for f in listing ])
-    
+
     # Check sorting (size)
     listing = c.get('/filebrowser/view' + BASE + '?sortby=size').context['files']
     assert_equal(expect, [ f['name'] for f in listing ])
@@ -599,27 +604,51 @@ def edit_helper(cluster, encoding, contents_pass_1, contents_pass_2):
 def test_upload():
   """Test file upload"""
   cluster = pseudo_hdfs4.shared_cluster()
+
   try:
-    USER_NAME = cluster.fs.superuser
-    cluster.fs.setuser(USER_NAME)
-    DEST = "/tmp/fb-upload-test"
+    USER_NAME = 'test'
+    USER_NAME_NOT_ME = 'not_me'
+    HDFS_DEST_DIR = "/tmp/fb-upload-test"
+    LOCAL_FILE = __file__
+    HDFS_FILE = HDFS_DEST_DIR + '/' + os.path.basename(__file__)
+
     client = make_logged_in_client(USER_NAME)
 
+    client_not_me = make_logged_in_client(username=USER_NAME_NOT_ME, is_superuser=False, groupname='test')
+    grant_access(USER_NAME_NOT_ME, "test", "filebrowser")
+
+    cluster.fs.do_as_superuser(cluster.fs.mkdir, HDFS_DEST_DIR)
+    cluster.fs.do_as_superuser(cluster.fs.chown, HDFS_DEST_DIR, USER_NAME, USER_NAME)
+    cluster.fs.do_as_superuser(cluster.fs.chmod, HDFS_DEST_DIR, 0700)
+
     # Just upload the current python file
     resp = client.post('/filebrowser/upload',
-                       dict(dest=DEST, hdfs_file=file(__file__)))
+                       dict(dest=HDFS_DEST_DIR, hdfs_file=file(LOCAL_FILE)))
+    response = json.loads(resp.content)
 
-    assert_true("View uploaded file" in resp.content)
-    stats = cluster.fs.stats(DEST)
+    assert_equal(0, response['status'], response)
+    stats = cluster.fs.stats(HDFS_FILE)
     assert_equal(stats['user'], USER_NAME)
     assert_equal(stats['group'], USER_NAME)
 
-    f = cluster.fs.open(DEST)
+    f = cluster.fs.open(HDFS_FILE)
     actual = f.read()
-    expected = file(__file__).read()
+    expected = file(LOCAL_FILE).read()
     assert_equal(actual, expected)
+
+    # Upload again and so fails because file already exits
+    resp = client.post('/filebrowser/upload',
+                       dict(dest=HDFS_DEST_DIR, hdfs_file=file(LOCAL_FILE)))
+    response = json.loads(resp.content)
+    assert_equal(-1, response['status'], response)
+
+    # Upload in tmp and fails because of missing permissions
+    resp = client_not_me.post('/filebrowser/upload',
+                              dict(dest=HDFS_DEST_DIR, hdfs_file=file(LOCAL_FILE)))
+    response = json.loads(resp.content)
+    assert_equal(-1, response['status'], response)
   finally:
     try:
-      cluster.fs.remove(DEST)
+      cluster.fs.remove(HDFS_DEST_DIR)
     except Exception, ex:
       pass

+ 16 - 3
desktop/libs/hadoop/src/hadoop/fs/webhdfs.py

@@ -111,7 +111,7 @@ class WebHdfs(Hdfs):
         self._superuser = DEFAULT_HDFS_SUPERUSER
 
     return self._superuser
-  
+
   @property
   def user(self):
     try:
@@ -341,7 +341,7 @@ class WebHdfs(Hdfs):
       if "out of the range" in ex.message:
         return ""
       raise ex
-      
+
 
   def open(self, path, mode='r'):
     """
@@ -399,7 +399,7 @@ class WebHdfs(Hdfs):
 
     CHUNK_SIZE = 65536
     offset = 0
-    
+
     while True:
       data = self.read(src, offset, CHUNK_SIZE)
       if offset == 0:
@@ -499,6 +499,19 @@ class WebHdfs(Hdfs):
     return res['Token']['urlString']
 
 
+  def do_as_user(self, username, fn, *args, **kwargs):
+    prev_user = self.user
+
+    try:
+      self.setuser(username)
+      fn(*args, **kwargs)
+    finally:
+      self.setuser(prev_user)
+
+
+  def do_as_superuser(self, fn, *args, **kwargs):
+    self.do_as_user(self.superuser, fn, *args, **kwargs)
+
 
 class File(object):
   """

+ 3 - 2
desktop/libs/liboozie/src/liboozie/submittion.py

@@ -146,11 +146,12 @@ class Submission(object):
         self.fs.copyfile(file, self.fs.join(lib_path, self.fs.basename(file)))
 
   def _do_as(self, username, fn, *args, **kwargs):
-    curr_user = self.fs.setuser(username)
+    prev_user = self.fs.user
     try:
+      self.fs.setuser(username)
       fn(*args, **kwargs)
     finally:
-      self.fs.setuser(curr_user)
+      self.fs.setuser(prev_user)
 
   def remove_deployment_dir(self):
     """Delete the workflow deployment directory."""