You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jo...@apache.org on 2019/07/01 18:55:32 UTC

[incubator-superset] branch master updated: [fix] Handling of non-existent datasource (#7755)

This is an automated email from the ASF dual-hosted git repository.

johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new e0d040c  [fix] Handling of non-existent datasource (#7755)
e0d040c is described below

commit e0d040c377b3d7252fe223bde37b7164e7a951c4
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Mon Jul 1 11:55:25 2019 -0700

    [fix] Handling of non-existent datasource (#7755)
---
 superset/views/core.py  | 44 ++++++++++++++++++++++++++++++++------------
 superset/views/utils.py | 32 ++++++++++++++++++++++++++------
 tests/core_tests.py     |  8 ++++++++
 3 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/superset/views/core.py b/superset/views/core.py
index cc0690b..a75eae4 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -64,7 +64,7 @@ from superset import (
 )
 from superset.connectors.connector_registry import ConnectorRegistry
 from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable
-from superset.exceptions import SupersetException
+from superset.exceptions import SupersetException, SupersetSecurityException
 from superset.forms import CsvToDatabaseForm
 from superset.jinja_context import get_template_processor
 from superset.legacy import update_time_range
@@ -134,24 +134,36 @@ def is_owner(obj, user):
     return obj and user in obj.owners
 
 
-def check_datasource_perms(self, datasource_type=None, datasource_id=None):
+def check_datasource_perms(
+    self, datasource_type: str = None, datasource_id: int = None
+) -> None:
     """
     Check if user can access a cached response from explore_json.
 
     This function takes `self` since it must have the same signature as the
     the decorated method.
 
+    :param datasource_type: The datasource type, i.e., 'druid' or 'table'
+    :param datasource_id: The datasource ID
+    :raises SupersetSecurityException: If the user cannot access the resource
     """
+
     form_data = get_form_data()[0]
-    datasource_id, datasource_type = get_datasource_info(
-        datasource_id, datasource_type, form_data
-    )
+
+    try:
+        datasource_id, datasource_type = get_datasource_info(
+            datasource_id, datasource_type, form_data
+        )
+    except SupersetException as e:
+        raise SupersetSecurityException(str(e))
+
     viz_obj = get_viz(
         datasource_type=datasource_type,
         datasource_id=datasource_id,
         form_data=form_data,
         force=False,
     )
+
     security_manager.assert_datasource_permission(viz_obj.datasource)
 
 
@@ -1403,9 +1415,14 @@ class Superset(BaseSupersetView):
         force = request.args.get("force") == "true"
 
         form_data = get_form_data()[0]
-        datasource_id, datasource_type = get_datasource_info(
-            datasource_id, datasource_type, form_data
-        )
+
+        try:
+            datasource_id, datasource_type = get_datasource_info(
+                datasource_id, datasource_type, form_data
+            )
+        except SupersetException as e:
+            return json_error_response(utils.error_msg_from_exception(e))
+
         viz_obj = get_viz(
             datasource_type=datasource_type,
             datasource_id=datasource_id,
@@ -1449,12 +1466,15 @@ class Superset(BaseSupersetView):
     def explore(self, datasource_type=None, datasource_id=None):
         user_id = g.user.get_id() if g.user else None
         form_data, slc = get_form_data(use_slice_data=True)
+        error_redirect = "/chart/list/"
 
-        datasource_id, datasource_type = get_datasource_info(
-            datasource_id, datasource_type, form_data
-        )
+        try:
+            datasource_id, datasource_type = get_datasource_info(
+                datasource_id, datasource_type, form_data
+            )
+        except SupersetException:
+            return redirect(error_redirect)
 
-        error_redirect = "/chart/list/"
         datasource = ConnectorRegistry.get_datasource(
             datasource_type, datasource_id, db.session
         )
diff --git a/superset/views/utils.py b/superset/views/utils.py
index 8ee0e1c..97660c5 100644
--- a/superset/views/utils.py
+++ b/superset/views/utils.py
@@ -16,7 +16,7 @@
 # under the License.
 # pylint: disable=C,R,W
 from collections import defaultdict
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Optional, Tuple
 from urllib import parse
 
 from flask import g, request
@@ -25,6 +25,7 @@ import simplejson as json
 
 from superset import app, db, viz
 from superset.connectors.connector_registry import ConnectorRegistry
+from superset.exceptions import SupersetException
 from superset.legacy import update_time_range
 import superset.models.core as models
 from superset.utils.core import QueryStatus
@@ -144,20 +145,39 @@ def get_form_data(slice_id=None, use_slice_data=False):
     return form_data, slc
 
 
-def get_datasource_info(datasource_id, datasource_type, form_data):
-    """Compatibility layer for handling of datasource info
+def get_datasource_info(
+    datasource_id: Optional[int],
+    datasource_type: Optional[str],
+    form_data: Dict[str, Any],
+) -> Tuple[int, Optional[str]]:
+    """
+    Compatibility layer for handling of datasource info
 
     datasource_id & datasource_type used to be passed in the URL
     directory, now they should come as part of the form_data,
-    This function allows supporting both without duplicating code"""
+
+    This function allows supporting both without duplicating code
+
+    :param datasource_id: The datasource ID
+    :param datasource_type: The datasource type, i.e., 'druid' or 'table'
+    :param form_data: The URL form data
+    :returns: The datasource ID and type
+    :raises SupersetException: If the datasource no longer exists
+    """
+
     datasource = form_data.get("datasource", "")
+
     if "__" in datasource:
         datasource_id, datasource_type = datasource.split("__")
         # The case where the datasource has been deleted
-        datasource_id = None if datasource_id == "None" else datasource_id
+        if datasource_id == "None":
+            datasource_id = None
 
     if not datasource_id:
-        raise Exception("The datasource associated with this chart no longer exists")
+        raise SupersetException(
+            "The datasource associated with this chart no longer exists"
+        )
+
     datasource_id = int(datasource_id)
     return datasource_id, datasource_type
 
diff --git a/tests/core_tests.py b/tests/core_tests.py
index e6b7cc1..2fdd6ad 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -727,6 +727,14 @@ class CoreTests(SupersetTestCase):
         self.assertEqual(data["status"], None)
         self.assertEqual(data["error"], None)
 
+    def test_slice_payload_no_datasource(self):
+        self.login(username="admin")
+        data = self.get_json_resp("/superset/explore_json/", raise_on_error=False)
+
+        self.assertEqual(
+            data["error"], "The datasource associated with this chart no longer exists"
+        )
+
     @mock.patch("superset.security.SupersetSecurityManager.schemas_accessible_by_user")
     @mock.patch("superset.security.SupersetSecurityManager.database_access")
     @mock.patch("superset.security.SupersetSecurityManager.all_datasource_access")