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

[notebook] Update historify API and add tests

use doc2 permissions decorator too
Jenny Kim 9 жил өмнө
parent
commit
f4292dc

+ 24 - 18
desktop/libs/notebook/src/notebook/api.py

@@ -22,12 +22,11 @@ from django.http import HttpResponseBadRequest, HttpResponseRedirect
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_GET, require_POST
 
-from desktop.decorators import check_document_access_permission
 from desktop.lib.django_util import JsonResponse
 from desktop.models import Document2, Document
 
 from notebook.connectors.base import get_api, Notebook, QueryExpired
-from notebook.decorators import api_error_handler, check_document_modify_permission
+from notebook.decorators import api_error_handler, check_document_access_permission, check_document_modify_permission
 from notebook.github import GithubClient
 from notebook.models import escape_rows
 
@@ -228,26 +227,30 @@ def save_notebook(request):
 
 
 @require_POST
-@check_document_modify_permission()
+@api_error_handler
+@check_document_access_permission()
 def historify(request):
   response = {'status': -1}
 
-  history = json.loads(request.POST.get('notebook', '{}'))
-  query_type = history['type']
-
-  history_doc = Document2.objects.create(name=history['name'], type=query_type, owner=request.user, is_history=True)
-  Document.objects.link(history_doc, owner=history_doc.owner, name=history_doc.name, description=history_doc.description, extra=query_type)
-
-  history_doc1 = history_doc.doc.get()
-  history_doc.update_data(history)
-  history_doc.name = history_doc1.name = history['name']
-  history_doc.description = history_doc1.description = history.get('description', '')
-  history_doc.is_history = True
+  notebook = json.loads(request.POST.get('notebook', '{}'))
+  query_type = notebook['type']
+
+  history_doc = Document2.objects.create(
+    name=notebook['name'],
+    type=query_type,
+    owner=request.user,
+    is_history=True
+  )
+  Document.objects.link(
+    history_doc,
+    name=history_doc.name,
+    owner=history_doc.owner,
+    description=history_doc.description,
+    extra=query_type
+  )
+
+  history_doc.update_data(notebook)
   history_doc.save()
-  history_doc1.save()
-
-  if history.get('id'): # If we come from a saved query
-    Document2.objects.get(id=history['id']).dependencies.add(history_doc)
 
   response['status'] = 0
   response['id'] = history_doc.id
@@ -257,6 +260,8 @@ def historify(request):
 
 
 @require_GET
+@api_error_handler
+@check_document_access_permission()
 def get_history(request):
   response = {'status': -1}
 
@@ -275,6 +280,7 @@ def get_history(request):
 
 
 @require_POST
+@api_error_handler
 @check_document_modify_permission()
 def clear_history(request):
   response = {'status': -1}

+ 2 - 2
desktop/libs/notebook/src/notebook/decorators.py

@@ -44,7 +44,7 @@ def check_document_access_permission():
       try:
         if notebook_id:
           document = Document2.objects.get(id=notebook_id)
-          document.doc.get().can_read_or_exception(request.user)
+          document.can_read_or_exception(request.user)
       except Document2.DoesNotExist:
         raise PopupException(_('Document %(id)s does not exist') % {'id': notebook_id})
 
@@ -61,7 +61,7 @@ def check_document_modify_permission():
       try:
         if notebook.get('id'):
           doc2 = Document2.objects.get(id=notebook['id'])
-          doc2.doc.get().can_write_or_exception(request.user)
+          doc2.can_write_or_exception(request.user)
       except Document.DoesNotExist:
         raise PopupException(_('Document %(id)s does not exist') % {'id': notebook.get('id')})
 

+ 87 - 0
desktop/libs/notebook/src/notebook/tests.py

@@ -15,11 +15,97 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
+
 from nose.tools import assert_equal, assert_true, assert_false
 
+from django.contrib.auth.models import User
+from django.core.urlresolvers import reverse
+
+from desktop.lib.django_test_util import make_logged_in_client
+from desktop.lib.test_utils import grant_access
+from desktop.models import Document, Document2
 from notebook.connectors.spark_shell import SparkApi
 
 
+class TestNotebookApi(object):
+
+  def setUp(self):
+    self.client = make_logged_in_client(username="test", groupname="default", recreate=True, is_superuser=False)
+    self.client_not_me = make_logged_in_client(username="not_perm_user", groupname="default", recreate=True, is_superuser=False)
+
+    self.user = User.objects.get(username="test")
+    self.user_not_me = User.objects.get(username="not_perm_user")
+
+    self.notebook_json = """
+      {
+        "selectedSnippet": "hive",
+        "showHistory": false,
+        "description": "Test Hive Query",
+        "name": "Test Hive Query",
+        "sessions": [
+            {
+                "type": "hive",
+                "properties": [],
+                "id": null
+            }
+        ],
+        "type": "query-hive",
+        "id": 50010,
+        "snippets": [],
+        "uuid": "5982a274-de78-083c-2efc-74f53dce744c"
+    }
+    """
+
+    self.notebook = json.loads(self.notebook_json)
+    self.doc2 = Document2.objects.create(id=50010, name=self.notebook['name'], type=self.notebook['type'], owner=self.user)
+    self.doc1 = Document.objects.link(self.doc2, owner=self.user, name=self.doc2.name,
+                                      description=self.doc2.description, extra=self.doc2.type)
+
+
+  def test_historify(self):
+    # Test that only users with access permissions can create a history doc
+    response = self.client_not_me.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    data = json.loads(response.content)
+    assert_equal(-1, data['status'], data)
+
+    # Test that historify creates new Doc2 and linked Doc1
+    assert_equal(0, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+    assert_equal(1, Document.objects.filter(name__contains=self.notebook['name']).count())
+
+    response = self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    data = json.loads(response.content)
+    assert_equal(0, data['status'], data)
+    assert_equal(1, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+    assert_equal(2, Document.objects.filter(name__contains=self.notebook['name']).count())
+
+    # TODO: test that shared query history saves with owner=current user
+
+
+  def test_get_history(self):
+    assert_equal(0, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    assert_equal(3, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+
+    # TODO: test that query history for shared query only returns docs accessible by current user
+
+
+  def test_clear_history(self):
+    assert_equal(0, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    self.client.post(reverse('notebook:historify'), {'notebook': self.notebook_json})
+    assert_equal(3, Document2.objects.filter(name__contains=self.notebook['name'], is_history=True).count())
+
+    # clear history should retain original document but wipe history
+    response = self.client.post(reverse('notebook:clear_history'), {'notebook': self.notebook_json})
+    data = json.loads(response.content)
+    assert_equal(0, data['status'], data)
+    assert_true(Document2.objects.filter(name__contains=self.notebook['name'], is_history=False).exists())
+
+
 class TestSparkShellConnector(object):
 
   LIVY_STANDALONE_LOG = """
@@ -156,6 +242,7 @@ class TestSparkShellConnector(object):
     self.user = 'hue_test'
     self.api = SparkApi(self.user)
 
+
   def test_get_jobs(self):
     local_jobs = [
       {'url': u'http://172.21.1.246:4040/jobs/job/?id=0', 'name': u'0'}