From 1ef7fb61785619e8e53ed60de9ea528384c740be Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Tue, 5 Mar 2019 01:11:59 -0800 Subject: [PATCH 01/86] Sparkline dates aren't formatting in Time Series Table (#6976) * Exclude venv for python linter to ignore * Fix NaN error --- superset/assets/src/visualizations/TimeTable/TimeTable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx index 38b9d2244e91f..24ebc80dd49e1 100644 --- a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx +++ b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx @@ -148,7 +148,7 @@ class TimeTable extends React.PureComponent { renderTooltip={({ index }) => (
{formatNumber(column.d3format, sparkData[index])} -
{formatTime(column.dateFormat, entries[index].time)}
+
{formatTime(column.dateFormat, new Date(entries[index].time))}
)} /> From e7d97db2fbec9b71402560f49ed5285593bbdee6 Mon Sep 17 00:00:00 2001 From: Christine Chambers Date: Wed, 13 Mar 2019 10:14:52 -0700 Subject: [PATCH 02/86] Fix the white background shown in SQL editor on drag (#7021) This PR sets the background-color css property on `.ace_scroller` instead of `.ace_content` to prevent the white background shown during resizing of the SQL editor before drag ends. --- superset/assets/src/SqlLab/main.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/SqlLab/main.less b/superset/assets/src/SqlLab/main.less index 28755a37c3266..6c35ed9c4fda6 100644 --- a/superset/assets/src/SqlLab/main.less +++ b/superset/assets/src/SqlLab/main.less @@ -294,7 +294,7 @@ div.tablePopover:hover { margin-top: 5px; } -.ace_content { +.ace_scroller { background-color: #f4f4f4; } From e6886fbf1fc4c5656f8df61f829e2b61d3e4ef20 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 22:28:12 +0200 Subject: [PATCH 03/86] Show tooltip with time frame (#6979) --- .../components/controls/DateFilterControl.jsx | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/superset/assets/src/explore/components/controls/DateFilterControl.jsx b/superset/assets/src/explore/components/controls/DateFilterControl.jsx index 9e07b99037cac..b068945bb6a79 100644 --- a/superset/assets/src/explore/components/controls/DateFilterControl.jsx +++ b/superset/assets/src/explore/components/controls/DateFilterControl.jsx @@ -32,6 +32,7 @@ import { Radio, Tab, Tabs, + Tooltip, } from 'react-bootstrap'; import Datetime from 'react-datetime'; import 'react-datetime/css/react-datetime.css'; @@ -311,15 +312,30 @@ export default class DateFilterControl extends React.Component { {grain} )); - const timeFrames = COMMON_TIME_FRAMES.map(timeFrame => ( - this.setState(getStateFromCommonTimeFrame(timeFrame))} - > - {timeFrame} - - )); + const timeFrames = COMMON_TIME_FRAMES.map((timeFrame) => { + const nextState = getStateFromCommonTimeFrame(timeFrame); + return ( + + {nextState.since}
{nextState.until} + + } + > +
+ this.setState(nextState)} + > + {timeFrame} + +
+
+ ); + }); return (
From b3966e4d7dc582db069143bad5ef28e2267165fe Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 22:28:44 +0200 Subject: [PATCH 04/86] Fix time filter control (#6978) --- superset/assets/src/explore/controls.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index b5589565eb235..cb5909a9934ff 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -914,7 +914,7 @@ export const controls = { 'The options here are defined on a per database ' + 'engine basis in the Superset source code.'), mapStateToProps: state => ({ - choices: (state.datasource) ? state.datasource.time_grain_sqla : null, + choices: (state.datasource) ? state.datasource.timeGrainSqla : null, }), }, From f4e392376e47af48db3b51464469228deb8c4a55 Mon Sep 17 00:00:00 2001 From: Conglei Date: Mon, 4 Mar 2019 18:06:59 -0800 Subject: [PATCH 05/86] Enhancement of query context and object. (#6962) * added more functionalities for query context and object. * fixed cache logic * added default value for groupby * updated comments and removed print (cherry picked from commit d5b9795f87f79fa2c41e144ffc00fd9586be7657) --- superset/common/query_context.py | 223 ++++++++++++++++++++++++++++++- superset/common/query_object.py | 86 ++++++++++-- superset/views/api.py | 44 ++++-- 3 files changed, 328 insertions(+), 25 deletions(-) diff --git a/superset/common/query_context.py b/superset/common/query_context.py index 0964292d8fd1b..5053372412509 100644 --- a/superset/common/query_context.py +++ b/superset/common/query_context.py @@ -14,30 +14,247 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# pylint: disable=R +# pylint: disable=C,R,W +from datetime import datetime, timedelta +import logging +import pickle as pkl +import traceback from typing import Dict, List +import numpy as np +import pandas as pd + +from superset import app, cache from superset import db from superset.connectors.connector_registry import ConnectorRegistry +from superset.utils import core as utils +from superset.utils.core import DTTM_ALIAS from .query_object import QueryObject +config = app.config +stats_logger = config.get('STATS_LOGGER') + class QueryContext: """ The query context contains the query object and additional fields necessary to retrieve the data payload for a given viz. """ + + default_fillna = 0 + cache_type = 'df' + enforce_numerical_metrics = True + # TODO: Type datasource and query_object dictionary with TypedDict when it becomes # a vanilla python type https://github.com/python/mypy/issues/5288 def __init__( self, datasource: Dict, queries: List[Dict], + force: bool = False, + custom_cache_timeout: int = None, ): self.datasource = ConnectorRegistry.get_datasource(datasource.get('type'), int(datasource.get('id')), db.session) self.queries = list(map(lambda query_obj: QueryObject(**query_obj), queries)) - def get_data(self): - raise NotImplementedError() + self.force = force + + self.custom_cache_timeout = custom_cache_timeout + + self.enforce_numerical_metrics = True + + def get_query_result(self, query_object): + """Returns a pandas dataframe based on the query object""" + + # Here, we assume that all the queries will use the same datasource, which is + # is a valid assumption for current setting. In a long term, we may or maynot + # support multiple queries from different data source. + + timestamp_format = None + if self.datasource.type == 'table': + dttm_col = self.datasource.get_col(query_object.granularity) + if dttm_col: + timestamp_format = dttm_col.python_date_format + + # The datasource here can be different backend but the interface is common + result = self.datasource.query(query_object.to_dict()) + + df = result.df + # Transform the timestamp we received from database to pandas supported + # datetime format. If no python_date_format is specified, the pattern will + # be considered as the default ISO date format + # If the datetime format is unix, the parse will use the corresponding + # parsing logic + if df is not None and not df.empty: + if DTTM_ALIAS in df.columns: + if timestamp_format in ('epoch_s', 'epoch_ms'): + # Column has already been formatted as a timestamp. + df[DTTM_ALIAS] = df[DTTM_ALIAS].apply(pd.Timestamp) + else: + df[DTTM_ALIAS] = pd.to_datetime( + df[DTTM_ALIAS], utc=False, format=timestamp_format) + if self.datasource.offset: + df[DTTM_ALIAS] += timedelta(hours=self.datasource.offset) + df[DTTM_ALIAS] += query_object.time_shift + + if self.enforce_numerical_metrics: + self.df_metrics_to_num(df, query_object) + + df.replace([np.inf, -np.inf], np.nan) + df = self.handle_nulls(df) + return { + 'query': result.query, + 'status': result.status, + 'error_message': result.error_message, + 'df': df, + } + + def df_metrics_to_num(self, df, query_object): + """Converting metrics to numeric when pandas.read_sql cannot""" + metrics = [metric for metric in query_object.metrics] + for col, dtype in df.dtypes.items(): + if dtype.type == np.object_ and col in metrics: + df[col] = pd.to_numeric(df[col], errors='coerce') + + def handle_nulls(self, df): + fillna = self.get_fillna_for_columns(df.columns) + return df.fillna(fillna) + + def get_fillna_for_col(self, col): + """Returns the value to use as filler for a specific Column.type""" + if col and col.is_string: + return ' NULL' + return self.default_fillna + + def get_fillna_for_columns(self, columns=None): + """Returns a dict or scalar that can be passed to DataFrame.fillna""" + if columns is None: + return self.default_fillna + columns_dict = {col.column_name: col for col in self.datasource.columns} + fillna = { + c: self.get_fillna_for_col(columns_dict.get(c)) + for c in columns + } + return fillna + + def get_data(self, df): + return df.to_dict(orient='records') + + def get_single_payload(self, query_obj): + """Returns a payload of metadata and data""" + payload = self.get_df_payload(query_obj) + df = payload.get('df') + status = payload.get('status') + if status != utils.QueryStatus.FAILED: + if df is not None and df.empty: + payload['error'] = 'No data' + else: + payload['data'] = self.get_data(df) + if 'df' in payload: + del payload['df'] + return payload + + def get_payload(self): + """Get all the paylaods from the arrays""" + return [self.get_single_payload(query_ojbect) for query_ojbect in self.queries] + + @property + def cache_timeout(self): + if self.custom_cache_timeout is not None: + return self.custom_cache_timeout + if self.datasource.cache_timeout is not None: + return self.datasource.cache_timeout + if ( + hasattr(self.datasource, 'database') and + self.datasource.database.cache_timeout) is not None: + return self.datasource.database.cache_timeout + return config.get('CACHE_DEFAULT_TIMEOUT') + + def get_df_payload(self, query_obj, **kwargs): + """Handles caching around the df paylod retrieval""" + cache_key = query_obj.cache_key( + datasource=self.datasource.uid, **kwargs) if query_obj else None + logging.info('Cache key: {}'.format(cache_key)) + is_loaded = False + stacktrace = None + df = None + cached_dttm = datetime.utcnow().isoformat().split('.')[0] + cache_value = None + status = None + query = '' + error_message = None + if cache_key and cache and not self.force: + cache_value = cache.get(cache_key) + if cache_value: + stats_logger.incr('loaded_from_cache') + try: + cache_value = pkl.loads(cache_value) + df = cache_value['df'] + query = cache_value['query'] + status = utils.QueryStatus.SUCCESS + is_loaded = True + except Exception as e: + logging.exception(e) + logging.error('Error reading cache: ' + + utils.error_msg_from_exception(e)) + logging.info('Serving from cache') + + if query_obj and not is_loaded: + try: + query_result = self.get_query_result(query_obj) + status = query_result['status'] + query = query_result['query'] + error_message = query_result['error_message'] + df = query_result['df'] + if status != utils.QueryStatus.FAILED: + stats_logger.incr('loaded_from_source') + is_loaded = True + except Exception as e: + logging.exception(e) + if not error_message: + error_message = '{}'.format(e) + status = utils.QueryStatus.FAILED + stacktrace = traceback.format_exc() + + if ( + is_loaded and + cache_key and + cache and + status != utils.QueryStatus.FAILED): + try: + cache_value = dict( + dttm=cached_dttm, + df=df if df is not None else None, + query=query, + ) + cache_value = pkl.dumps( + cache_value, protocol=pkl.HIGHEST_PROTOCOL) + + logging.info('Caching {} chars at key {}'.format( + len(cache_value), cache_key)) + + stats_logger.incr('set_cache_key') + cache.set( + cache_key, + cache_value, + timeout=self.cache_timeout) + except Exception as e: + # cache.set call can fail if the backend is down or if + # the key is too large or whatever other reasons + logging.warning('Could not cache key {}'.format(cache_key)) + logging.exception(e) + cache.delete(cache_key) + return { + 'cache_key': cache_key, + 'cached_dttm': cache_value['dttm'] if cache_value is not None else None, + 'cache_timeout': self.cache_timeout, + 'df': df, + 'error': error_message, + 'is_cached': cache_key is not None, + 'query': query, + 'status': status, + 'stacktrace': stacktrace, + 'rowcount': len(df.index) if df is not None else 0, + } diff --git a/superset/common/query_object.py b/superset/common/query_object.py index c851d47c16428..a2394042a09f8 100644 --- a/superset/common/query_object.py +++ b/superset/common/query_object.py @@ -15,15 +15,17 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=R -from typing import Dict, List, Optional +import hashlib +from typing import Dict, List, Optional, Union + +import simplejson as json from superset import app from superset.utils import core as utils + # TODO: Type Metrics dictionary with TypedDict when it becomes a vanilla python type # https://github.com/python/mypy/issues/5288 -Metric = Dict - class QueryObject: """ @@ -33,31 +35,87 @@ class QueryObject: def __init__( self, granularity: str, + metrics: List[Union[Dict, str]], groupby: List[str] = None, - metrics: List[Metric] = None, filters: List[str] = None, time_range: Optional[str] = None, time_shift: Optional[str] = None, is_timeseries: bool = False, + timeseries_limit: int = 0, row_limit: int = app.config.get('ROW_LIMIT'), - limit: int = 0, - timeseries_limit_metric: Optional[Metric] = None, + timeseries_limit_metric: Optional[Dict] = None, order_desc: bool = True, extras: Optional[Dict] = None, + prequeries: Optional[Dict] = None, + is_prequery: bool = False, + columns: List[str] = None, + orderby: List[List] = None, ): self.granularity = granularity self.from_dttm, self.to_dttm = utils.get_since_until(time_range, time_shift) self.is_timeseries = is_timeseries - self.groupby = groupby or [] - self.metrics = metrics or [] - self.filter = filters or [] + self.time_range = time_range + self.time_shift = utils.parse_human_timedelta(time_shift) + self.groupby = groupby if groupby is not None else [] + + # Temporal solution for backward compatability issue + # due the new format of non-ad-hoc metric. + self.metrics = [metric if 'expressionType' in metric else metric['label'] + for metric in metrics] self.row_limit = row_limit - self.timeseries_limit = int(limit) + self.filter = filters if filters is not None else [] + self.timeseries_limit = timeseries_limit self.timeseries_limit_metric = timeseries_limit_metric self.order_desc = order_desc - self.prequeries = [] - self.is_prequery = False - self.extras = extras + self.prequeries = prequeries + self.is_prequery = is_prequery + self.extras = extras if extras is not None else {} + self.columns = columns if columns is not None else [] + self.orderby = orderby if orderby is not None else [] def to_dict(self): - raise NotImplementedError() + query_object_dict = { + 'granularity': self.granularity, + 'from_dttm': self.from_dttm, + 'to_dttm': self.to_dttm, + 'is_timeseries': self.is_timeseries, + 'groupby': self.groupby, + 'metrics': self.metrics, + 'row_limit': self.row_limit, + 'filter': self.filter, + 'timeseries_limit': self.timeseries_limit, + 'timeseries_limit_metric': self.timeseries_limit_metric, + 'order_desc': self.order_desc, + 'prequeries': self.prequeries, + 'is_prequery': self.is_prequery, + 'extras': self.extras, + 'columns': self.columns, + 'orderby': self.orderby, + } + return query_object_dict + + def cache_key(self, **extra): + """ + The cache key is made out of the key/values in `query_obj`, plus any + other key/values in `extra` + We remove datetime bounds that are hard values, and replace them with + the use-provided inputs to bounds, which may be time-relative (as in + "5 days ago" or "now"). + """ + cache_dict = self.to_dict() + cache_dict.update(extra) + + for k in ['from_dttm', 'to_dttm']: + del cache_dict[k] + if self.time_range: + cache_dict['time_range'] = self.time_range + json_data = self.json_dumps(cache_dict, sort_keys=True) + return hashlib.md5(json_data.encode('utf-8')).hexdigest() + + def json_dumps(self, obj, sort_keys=False): + return json.dumps( + obj, + default=utils.json_int_dttm_ser, + ignore_nan=True, + sort_keys=sort_keys, + ) diff --git a/superset/views/api.py b/superset/views/api.py index 7b84217c55d24..aadee9cb46d9a 100644 --- a/superset/views/api.py +++ b/superset/views/api.py @@ -15,16 +15,18 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=R -import json - -from flask import g, request +from flask import request from flask_appbuilder import expose from flask_appbuilder.security.decorators import has_access_api +import simplejson as json -from superset import appbuilder, security_manager +from superset import appbuilder, db, security_manager from superset.common.query_context import QueryContext +from superset.legacy import update_time_range +import superset.models.core as models from superset.models.core import Log -from .base import api, BaseSupersetView, data_payload_response, handle_api_exception +from superset.utils import core as utils +from .base import api, BaseSupersetView, handle_api_exception class Api(BaseSupersetView): @@ -37,11 +39,37 @@ def query(self): """ Takes a query_obj constructed in the client and returns payload data response for the given query_obj. + params: query_context: json_blob """ query_context = QueryContext(**json.loads(request.form.get('query_context'))) - security_manager.assert_datasource_permission(query_context.datasource, g.user) - payload_json = query_context.get_data() - return data_payload_response(payload_json) + security_manager.assert_datasource_permission(query_context.datasource) + payload_json = query_context.get_payload() + return json.dumps( + payload_json, + default=utils.json_int_dttm_ser, + ignore_nan=True, + ) + + @Log.log_this + @api + @handle_api_exception + @has_access_api + @expose('/v1/form_data/', methods=['GET']) + def query_form_data(self): + """ + Get the formdata stored in the database for existing slice. + params: slice_id: integer + """ + form_data = {} + slice_id = request.args.get('slice_id') + if slice_id: + slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() + if slc: + form_data = slc.form_data.copy() + + update_time_range(form_data) + + return json.dumps(form_data) appbuilder.add_view_no_menu(Api) From d7b2c3ebf5a11b6fd11e34e5ad34e94363ebaa2e Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Sat, 9 Mar 2019 20:49:08 -0800 Subject: [PATCH 06/86] [fix] /superset/slice/id url is too long (#6989) (cherry picked from commit 6a4d507ab607b01ed324cb3341b71c6fb2cb5c97) --- superset/views/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/views/core.py b/superset/views/core.py index 48f8324737dde..06c6925c3bf5e 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1111,7 +1111,7 @@ def slice(self, slice_id): if not slc: abort(404) endpoint = '/superset/explore/?form_data={}'.format( - parse.quote(json.dumps(form_data)), + parse.quote(json.dumps({'slice_id': slice_id})), ) if request.args.get('standalone') == 'true': endpoint += '&standalone=true' From 9855837e0ca62906d2825677b8a91bb0e2b7ee5f Mon Sep 17 00:00:00 2001 From: Tom Hunter Date: Thu, 14 Mar 2019 14:20:22 -0400 Subject: [PATCH 07/86] [WIP] fix user specified JSON metadata not updating dashboard on refresh (#7027) (cherry picked from commit cc58f0e661044e95c7c86d0da8d77a0a6640efe7) --- superset/views/core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/views/core.py b/superset/views/core.py index 06c6925c3bf5e..78044c63a4e97 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -633,8 +633,9 @@ class DashboardModelView(SupersetModelView, DeleteMixin): # noqa } def pre_add(self, obj): - obj.slug = obj.slug.strip() or None + obj.slug = obj.slug or None if obj.slug: + obj.slug = obj.slug.strip() obj.slug = obj.slug.replace(' ', '-') obj.slug = re.sub(r'[^\w\-]+', '', obj.slug) if g.user not in obj.owners: From 6d0c3906f54f5dc11e20ec444c47cb2919b94224 Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Tue, 19 Mar 2019 14:26:17 -0700 Subject: [PATCH 08/86] feat: add ability to change font size in big number (#7003) * Add ability to change font sizes in Big Number * rename big number to header * Add comment to clarify font size values --- superset/assets/backendSync.json | 58 +++++++++++++++++ .../src/explore/controlPanels/BigNumber.js | 13 +++- .../explore/controlPanels/BigNumberTotal.js | 13 +++- superset/assets/src/explore/controls.jsx | 62 +++++++++++++++++++ 4 files changed, 144 insertions(+), 2 deletions(-) diff --git a/superset/assets/backendSync.json b/superset/assets/backendSync.json index 8008621d42ddb..d334778d68d91 100644 --- a/superset/assets/backendSync.json +++ b/superset/assets/backendSync.json @@ -2493,6 +2493,64 @@ "default": "150", "description": "Font size for the biggest value in the list" }, + "header_font_size": { + "type": "SelectControl", + "label": "Header Font Size", + "renderTrigger": true, + "clearable": false, + "default": 0.3, + "options": [ + { + "label": "Tiny", + "value": 0.125 + }, + { + "label": "Small", + "value": 0.2 + }, + { + "label": "Normal", + "value": 0.3 + }, + { + "label": "Large", + "value": 0.4 + }, + { + "label": "Huge", + "value": 0.5 + } + ] + }, + "subheader_font_size": { + "type": "SelectControl", + "label": "Subheader Font Size", + "renderTrigger": true, + "clearable": false, + "default": 0.125, + "options": [ + { + "label": "Tiny", + "value": 0.125 + }, + { + "label": "Small", + "value": 0.2 + }, + { + "label": "Normal", + "value": 0.3 + }, + { + "label": "Large", + "value": 0.4 + }, + { + "label": "Huge", + "value": 0.5 + } + ] + }, "instant_filtering": { "type": "CheckboxControl", "label": "Instant Filtering", diff --git a/superset/assets/src/explore/controlPanels/BigNumber.js b/superset/assets/src/explore/controlPanels/BigNumber.js index b931c7100c496..e621ee1f74b84 100644 --- a/superset/assets/src/explore/controlPanels/BigNumber.js +++ b/superset/assets/src/explore/controlPanels/BigNumber.js @@ -29,13 +29,21 @@ export default { ], }, { - label: t('Chart Options'), + label: t('Options'), expanded: true, controlSetRows: [ ['compare_lag', 'compare_suffix'], ['y_axis_format', null], ['show_trend_line', 'start_y_axis_at_zero'], + ], + }, + { + label: t('Chart Options'), + expanded: true, + controlSetRows: [ ['color_picker', null], + ['header_font_size'], + ['subheader_font_size'], ], }, ], @@ -43,5 +51,8 @@ export default { y_axis_format: { label: t('Number format'), }, + header_font_size: { + label: t('Big Number Font Size'), + }, }, }; diff --git a/superset/assets/src/explore/controlPanels/BigNumberTotal.js b/superset/assets/src/explore/controlPanels/BigNumberTotal.js index 22a7d69805131..7b3e730ce5df3 100644 --- a/superset/assets/src/explore/controlPanels/BigNumberTotal.js +++ b/superset/assets/src/explore/controlPanels/BigNumberTotal.js @@ -29,17 +29,28 @@ export default { ], }, { - label: t('Chart Options'), + label: t('Options'), expanded: true, controlSetRows: [ ['subheader'], ['y_axis_format'], ], }, + { + label: t('Chart Options'), + expanded: true, + controlSetRows: [ + ['header_font_size'], + ['subheader_font_size'], + ], + }, ], controlOverrides: { y_axis_format: { label: t('Number format'), }, + header_font_size: { + label: t('Big Number Font Size'), + }, }, }; diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index cb5909a9934ff..d50db9c65c19f 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -1399,6 +1399,68 @@ export const controls = { description: t('Font size for the biggest value in the list'), }, + header_font_size: { + type: 'SelectControl', + label: t('Header Font Size'), + renderTrigger: true, + clearable: false, + default: 0.3, + // Values represent the percentage of space a header should take + options: [ + { + label: t('Tiny'), + value: 0.125, + }, + { + label: t('Small'), + value: 0.2, + }, + { + label: t('Normal'), + value: 0.3, + }, + { + label: t('Large'), + value: 0.4, + }, + { + label: t('Huge'), + value: 0.5, + }, + ], + }, + + subheader_font_size: { + type: 'SelectControl', + label: t('Subheader Font Size'), + renderTrigger: true, + clearable: false, + default: 0.125, + // Values represent the percentage of space a subheader should take + options: [ + { + label: t('Tiny'), + value: 0.125, + }, + { + label: t('Small'), + value: 0.2, + }, + { + label: t('Normal'), + value: 0.3, + }, + { + label: t('Large'), + value: 0.4, + }, + { + label: t('Huge'), + value: 0.5, + }, + ], + }, + instant_filtering: { type: 'CheckboxControl', label: t('Instant Filtering'), From 0e1a3d3fdd2e7ffb1e2c33005eccaaf16a28fb89 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 20 Mar 2019 16:56:30 -0700 Subject: [PATCH 09/86] Allow LIMIT to be specified in parameters (#7052) --- superset/views/core.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 78044c63a4e97..a250e6083ccf7 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2531,10 +2531,8 @@ def sql_json(self): ) client_id = request.form.get('client_id') or utils.shortid()[:10] - limits = [mydb.db_engine_spec.get_limit_from_sql(sql), limit] query = Query( database_id=int(database_id), - limit=min(lim for lim in limits if lim is not None), sql=sql, schema=schema, select_as_cta=request.form.get('select_as_cta') == 'true', @@ -2564,6 +2562,10 @@ def sql_json(self): return json_error_response( 'Template rendering failed: {}'.format(utils.error_msg_from_exception(e))) + # set LIMIT after template processing + limits = [mydb.db_engine_spec.get_limit_from_sql(rendered_query), limit] + query.limit = min(lim for lim in limits if lim is not None) + # Async request. if async_: logging.info('Running query on a Celery worker') From 0c65b3f17847692b7b307008c173e6f8da4eacae Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Fri, 15 Mar 2019 10:56:19 -0700 Subject: [PATCH 10/86] [fix] Cursor jumping when editing chart and dashboard titles (#7038) (cherry picked from commit fc1770f7b79a4d8815b646b46390fabf190c3815) --- .../assets/src/components/EditableTitle.jsx | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/superset/assets/src/components/EditableTitle.jsx b/superset/assets/src/components/EditableTitle.jsx index 87a5160caf86b..428f9953baf5b 100644 --- a/superset/assets/src/components/EditableTitle.jsx +++ b/superset/assets/src/components/EditableTitle.jsx @@ -55,7 +55,6 @@ export default class EditableTitle extends React.PureComponent { this.handleClick = this.handleClick.bind(this); this.handleBlur = this.handleBlur.bind(this); this.handleChange = this.handleChange.bind(this); - this.handleKeyUp = this.handleKeyUp.bind(this); this.handleKeyPress = this.handleKeyPress.bind(this); // Used so we can access the DOM element if a user clicks on this component. @@ -112,21 +111,16 @@ export default class EditableTitle extends React.PureComponent { } } - handleKeyUp(ev) { - // this entire method exists to support using EditableTitle as the title of a - // react-bootstrap Tab, as a workaround for this line in react-bootstrap https://goo.gl/ZVLmv4 - // - // tl;dr when a Tab EditableTitle is being edited, typically the Tab it's within has been - // clicked and is focused/active. for accessibility, when focused the Tab intercepts - // the ' ' key (among others, including all arrows) and onChange() doesn't fire. somehow - // keydown is still called so we can detect this and manually add a ' ' to the current title - if (ev.key === ' ') { - let title = ev.target.value; - const titleLength = (title || '').length; - if (title && title[titleLength - 1] !== ' ') { - title = `${title} `; - this.setState(() => ({ title })); - } + // this entire method exists to support using EditableTitle as the title of a + // react-bootstrap Tab, as a workaround for this line in react-bootstrap https://goo.gl/ZVLmv4 + // + // tl;dr when a Tab EditableTitle is being edited, typically the Tab it's within has been + // clicked and is focused/active. for accessibility, when focused the Tab intercepts + // the ' ' key (among others, including all arrows) and onChange() doesn't fire. somehow + // keydown is still called so we can detect this and manually add a ' ' to the current title + handleKeyDown(event) { + if (event.key === ' ') { + event.stopPropagation(); } } @@ -170,7 +164,7 @@ export default class EditableTitle extends React.PureComponent { required value={value} className={!title ? 'text-muted' : null} - onKeyUp={this.handleKeyUp} + onKeyDown={this.handleKeyDown} onChange={this.handleChange} onBlur={this.handleBlur} onClick={this.handleClick} @@ -184,7 +178,7 @@ export default class EditableTitle extends React.PureComponent { type={isEditing ? 'text' : 'button'} value={value} className={!title ? 'text-muted' : null} - onKeyUp={this.handleKeyUp} + onKeyDown={this.handleKeyDown} onChange={this.handleChange} onBlur={this.handleBlur} onClick={this.handleClick} From c65a1665b77dd838ec20fe4bbdd03079c18fb8df Mon Sep 17 00:00:00 2001 From: michellethomas Date: Wed, 13 Mar 2019 09:34:51 -0700 Subject: [PATCH 11/86] Changing time table viz to pass formatTime a date (#7020) (cherry picked from commit 7f3c145b1f5a4e2d8b95982119503e98772e2c47) --- superset/assets/src/visualizations/TimeTable/TimeTable.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx index 24ebc80dd49e1..c895d46187717 100644 --- a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx +++ b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx @@ -23,6 +23,7 @@ import { scaleLinear } from 'd3-scale'; import { Table, Thead, Th, Tr, Td } from 'reactable-arc'; import { formatNumber } from '@superset-ui/number-format'; import { formatTime } from '@superset-ui/time-format'; +import moment from 'moment'; import MetricOption from '../../components/MetricOption'; import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger'; @@ -148,7 +149,7 @@ class TimeTable extends React.PureComponent { renderTooltip={({ index }) => (
{formatNumber(column.d3format, sparkData[index])} -
{formatTime(column.dateFormat, new Date(entries[index].time))}
+
{formatTime(column.dateFormat, moment.utc(entries[index].time).toDate())}
)} /> From 771d212abb38747e89eab8bb7fbed53d61e42f88 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Wed, 13 Mar 2019 13:22:28 -0700 Subject: [PATCH 12/86] [db-engine-spec] Aligning Hive/Presto partition logic (#7007) (cherry picked from commit 05be86611785fef2904992e4e7d31dce23f1c51b) --- superset/db_engine_specs.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 6412d10442ebf..4bb15d8cc29cf 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -977,16 +977,16 @@ def where_latest_partition( except Exception: # table is not partitioned return False - for c in columns: - if c.get('name') == col_name: - return qry.where(Column(col_name) == value) + if value is not None: + for c in columns: + if c.get('name') == col_name: + return qry.where(Column(col_name) == value) return False @classmethod def _latest_partition_from_df(cls, df): - recs = df.to_records(index=False) - if recs: - return recs[0][0] + if not df.empty: + return df.to_records(index=False)[0][0] @classmethod def latest_partition(cls, table_name, schema, database, show_first=False): @@ -1003,7 +1003,7 @@ def latest_partition(cls, table_name, schema, database, show_first=False): :type show_first: bool >>> latest_partition('foo_table') - '2018-01-01' + ('ds', '2018-01-01') """ indexes = database.get_indexes(table_name, schema) if len(indexes[0]['column_names']) < 1: @@ -1321,9 +1321,10 @@ def where_latest_partition( except Exception: # table is not partitioned return False - for c in columns: - if c.get('name') == col_name: - return qry.where(Column(col_name) == value) + if value is not None: + for c in columns: + if c.get('name') == col_name: + return qry.where(Column(col_name) == value) return False @classmethod @@ -1334,7 +1335,8 @@ def latest_sub_partition(cls, table_name, schema, database, **kwargs): @classmethod def _latest_partition_from_df(cls, df): """Hive partitions look like ds={partition name}""" - return df.ix[:, 0].max().split('=')[1] + if not df.empty: + return df.ix[:, 0].max().split('=')[1] @classmethod def _partition_query( From fa73a8d244f293d94927730a5ff28dd1dd3ea87f Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Sat, 16 Mar 2019 10:44:14 -0700 Subject: [PATCH 13/86] [fix] explore chart from dashboard missed slice title (#7046) (cherry picked from commit a6d48d4052839286aec725d51303b3b2bf6e8dd4) --- superset/assets/src/dashboard/actions/dashboardState.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js index bdade32aedc1f..135658d9331b2 100644 --- a/superset/assets/src/dashboard/actions/dashboardState.js +++ b/superset/assets/src/dashboard/actions/dashboardState.js @@ -236,7 +236,10 @@ export function addSliceToDashboard(id) { ), ); } - const form_data = selectedSlice.form_data; + const form_data = { + ...selectedSlice.form_data, + slice_id: selectedSlice.slice_id, + }; const newChart = { ...initChart, id, From 8d6e3f1d8095f3ae8e8201e9855c0a0252bcbbdd Mon Sep 17 00:00:00 2001 From: Conglei Date: Thu, 14 Mar 2019 14:05:06 -0700 Subject: [PATCH 14/86] fix inaccurate data calculation with adata rolling and contribution (#7035) (cherry picked from commit 0782e831cd37f665a2838119d87c433269f1b36b) --- superset/viz.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/superset/viz.py b/superset/viz.py index de33a3ca8c8b3..ef7ee456ebe11 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -1184,10 +1184,6 @@ def process_data(self, df, aggregate=False): dfs.sort_values(ascending=False, inplace=True) df = df[dfs.index] - if fd.get('contribution'): - dft = df.T - df = (dft / dft.sum()).T - rolling_type = fd.get('rolling_type') rolling_periods = int(fd.get('rolling_periods') or 0) min_periods = int(fd.get('min_periods') or 0) @@ -1207,6 +1203,10 @@ def process_data(self, df, aggregate=False): if min_periods: df = df[min_periods:] + if fd.get('contribution'): + dft = df.T + df = (dft / dft.sum()).T + return df def run_extra_queries(self): From 6d0752cc8a35c648a8d48a3ba16903a844b824eb Mon Sep 17 00:00:00 2001 From: michellethomas Date: Mon, 18 Mar 2019 10:14:26 -0700 Subject: [PATCH 15/86] Adding warning message for sqllab save query (#7028) (cherry picked from commit ead3d48133e7e1ab8b91d51e561544a544b4eaad) --- .../assets/src/SqlLab/components/SaveQuery.jsx | 14 ++++++++++++++ .../assets/src/SqlLab/components/SqlEditor.jsx | 3 +++ .../src/SqlLab/components/TabbedSqlEditors.jsx | 4 ++++ superset/config.py | 3 +++ superset/views/base.py | 1 + 5 files changed, 25 insertions(+) diff --git a/superset/assets/src/SqlLab/components/SaveQuery.jsx b/superset/assets/src/SqlLab/components/SaveQuery.jsx index 4a3ac1ba5f120..6ae2508ec1666 100644 --- a/superset/assets/src/SqlLab/components/SaveQuery.jsx +++ b/superset/assets/src/SqlLab/components/SaveQuery.jsx @@ -31,11 +31,13 @@ const propTypes = { dbId: PropTypes.number, animation: PropTypes.bool, onSave: PropTypes.func, + saveQueryWarning: PropTypes.string, }; const defaultProps = { defaultLabel: t('Undefined'), animation: true, onSave: () => {}, + saveQueryWarning: null, }; class SaveQuery extends React.PureComponent { @@ -108,6 +110,18 @@ class SaveQuery extends React.PureComponent {
+ {this.props.saveQueryWarning && ( +
+ + + + {this.props.saveQueryWarning} + + + +
+
+ )}
@@ -289,6 +292,7 @@ function mapStateToProps({ sqlLab, common }) { offline: sqlLab.offline, defaultQueryLimit: common.conf.DEFAULT_SQLLAB_LIMIT, maxRow: common.conf.SQL_MAX_ROW, + saveQueryWarning: common.conf.SQLLAB_SAVE_WARNING_MESSAGE, }; } function mapDispatchToProps(dispatch) { diff --git a/superset/config.py b/superset/config.py index c44328e5a904a..d03a6bd6492f9 100644 --- a/superset/config.py +++ b/superset/config.py @@ -356,6 +356,9 @@ # Maximum number of tables/views displayed in the dropdown window in SQL Lab. MAX_TABLE_NAMES = 3000 +# Adds a warning message on sqllab save query modal. +SQLLAB_SAVE_WARNING_MESSAGE = None + # If defined, shows this text in an alert-warning box in the navbar # one example use case may be "STAGING" to make it clear that this is # not the production version of the site. diff --git a/superset/views/base.py b/superset/views/base.py index 4f3c8e8ae1e8b..071f2b335b658 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -43,6 +43,7 @@ 'DEFAULT_SQLLAB_LIMIT', 'SQL_MAX_ROW', 'SUPERSET_WEBSERVER_DOMAINS', + 'SQLLAB_SAVE_WARNING_MESSAGE', ) From bd2a8b5335687b712ebd91c4b858d9524bd32e91 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Mon, 18 Mar 2019 09:56:52 -0700 Subject: [PATCH 16/86] [datasource] Ensuring consistent behavior of datasource editing/saving. (#7037) * Update datasource.py * Update datasource.py (cherry picked from commit c771625f1068d3a7f41e6bced14b0cbdbf9962cc) --- superset/views/datasource.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/superset/views/datasource.py b/superset/views/datasource.py index eda4e8f7b668b..43e8c42a5ccab 100644 --- a/superset/views/datasource.py +++ b/superset/views/datasource.py @@ -20,12 +20,11 @@ from flask import request from flask_appbuilder import expose from flask_appbuilder.security.decorators import has_access_api -from flask_babel import gettext as __ from superset import appbuilder, db from superset.connectors.connector_registry import ConnectorRegistry from superset.models.core import Database -from .base import BaseSupersetView, check_ownership, json_error_response +from .base import BaseSupersetView, json_error_response class Datasource(BaseSupersetView): @@ -39,14 +38,6 @@ def save(self): orm_datasource = ConnectorRegistry.get_datasource( datasource_type, datasource_id, db.session) - if not check_ownership(orm_datasource, raise_if_false=False): - return json_error_response( - __( - 'You are not authorized to modify ' - 'this data source configuration'), - status='401', - ) - if 'owners' in datasource: datasource['owners'] = db.session.query(orm_datasource.owner_class).filter( orm_datasource.owner_class.id.in_(datasource['owners'])).all() From 81a1287f2f05cc10bd8e3654fdc3f21dfddc693a Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Sun, 3 Mar 2019 15:27:08 -0800 Subject: [PATCH 17/86] [csv-upload] Fixing message encoding (#6971) (cherry picked from commit 48431ab5b9375a94c5262a0336d9c69e5f01a3ac) --- superset/views/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/views/core.py b/superset/views/core.py index a250e6083ccf7..3ceec89f50577 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -401,7 +401,7 @@ def form_post(self, form): except OSError: pass message = 'Table name {} already exists. Please pick another'.format( - form.name.data) if isinstance(e, IntegrityError) else e + form.name.data) if isinstance(e, IntegrityError) else str(e) flash( message, 'danger') From 6a7d5fc45777dca2dd965a05c147b69702b59652 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Tue, 5 Mar 2019 09:36:08 -0800 Subject: [PATCH 18/86] [sql-parse] Fixing LIMIT exceptions (#6963) (cherry picked from commit 3e076cb60b385e675ed1c9a8053493375e43370b) --- superset/sql_parse.py | 19 ++++++++----------- tests/db_engine_specs_test.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/superset/sql_parse.py b/superset/sql_parse.py index d1ad23d97c9c8..b653c88a2a001 100644 --- a/superset/sql_parse.py +++ b/superset/sql_parse.py @@ -154,18 +154,15 @@ def __extract_from_token(self, token, depth=0): if not self.__is_identifier(token): self.__extract_from_token(item, depth=depth + 1) - def _get_limit_from_token(self, token): - if token.ttype == sqlparse.tokens.Literal.Number.Integer: - return int(token.value) - elif token.is_group: - return int(token.get_token_at_offset(1).value) - def _extract_limit_from_query(self, statement): - limit_token = None - for pos, item in enumerate(statement.tokens): - if item.ttype in Keyword and item.value.lower() == 'limit': - limit_token = statement.tokens[pos + 2] - return self._get_limit_from_token(limit_token) + idx, _ = statement.token_next_by(m=(Keyword, 'LIMIT')) + if idx is not None: + _, token = statement.token_next(idx=idx) + if token: + if isinstance(token, IdentifierList): + _, token = token.token_next(idx=-1) + if token and token.ttype == sqlparse.tokens.Literal.Number.Integer: + return int(token.value) def get_query_with_new_limit(self, new_limit): """returns the query with the specified limit""" diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index a48012d4c1c19..1390f5092a45c 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -142,12 +142,26 @@ def test_extract_limit_from_query(self, engine_spec_class=MySQLEngineSpec): q2 = 'select * from (select * from my_subquery limit 10) where col=1 limit 20' q3 = 'select * from (select * from my_subquery limit 10);' q4 = 'select * from (select * from my_subquery limit 10) where col=1 limit 20;' + q5 = 'select * from mytable limit 10, 20' + q6 = 'select * from mytable limit 10 offset 20' + q7 = 'select * from mytable limit' + q8 = 'select * from mytable limit 10.0' + q9 = 'select * from mytable limit x' + q10 = 'select * from mytable limit x, 20' + q11 = 'select * from mytable limit x offset 20' self.assertEqual(engine_spec_class.get_limit_from_sql(q0), None) self.assertEqual(engine_spec_class.get_limit_from_sql(q1), 10) self.assertEqual(engine_spec_class.get_limit_from_sql(q2), 20) self.assertEqual(engine_spec_class.get_limit_from_sql(q3), None) self.assertEqual(engine_spec_class.get_limit_from_sql(q4), 20) + self.assertEqual(engine_spec_class.get_limit_from_sql(q5), 10) + self.assertEqual(engine_spec_class.get_limit_from_sql(q6), 10) + self.assertEqual(engine_spec_class.get_limit_from_sql(q7), None) + self.assertEqual(engine_spec_class.get_limit_from_sql(q8), None) + self.assertEqual(engine_spec_class.get_limit_from_sql(q9), None) + self.assertEqual(engine_spec_class.get_limit_from_sql(q10), None) + self.assertEqual(engine_spec_class.get_limit_from_sql(q11), None) def test_wrapped_query(self): self.sql_limit_regex( From ee850892ed0ee80a9e6881d65c48a161b9a9dc7e Mon Sep 17 00:00:00 2001 From: michellethomas Date: Wed, 6 Mar 2019 15:04:04 -0800 Subject: [PATCH 19/86] Adding custom control overrides (#6956) * Adding extraOverrides to line chart * Updating extraOverrides to fit with more cases * Moving extraOverrides to index.js * Removing webpack-merge in package.json * Fixing metrics control clearing metric (cherry picked from commit e6194051f486e42922dc4e34a861f4490c1062fc) --- .../components/controls/MetricsControl.jsx | 31 ++++++++++++++++--- .../explore/controlPanels/extraOverrides.js | 22 +++++++++++++ .../assets/src/explore/controlPanels/index.js | 5 +-- 3 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 superset/assets/src/explore/controlPanels/extraOverrides.js diff --git a/superset/assets/src/explore/components/controls/MetricsControl.jsx b/superset/assets/src/explore/components/controls/MetricsControl.jsx index 02078ffbfc668..b42cc814d67b6 100644 --- a/superset/assets/src/explore/components/controls/MetricsControl.jsx +++ b/superset/assets/src/explore/components/controls/MetricsControl.jsx @@ -60,6 +60,21 @@ function isDictionaryForAdhocMetric(value) { return value && !(value instanceof AdhocMetric) && value.expressionType; } +function columnsContainAllMetrics(value, nextProps) { + const columnNames = new Set( + [...nextProps.columns, ...nextProps.savedMetrics] + // eslint-disable-next-line camelcase + .map(({ column_name, metric_name }) => (column_name || metric_name)), + ); + + return (Array.isArray(value) ? value : [value]) + .filter(metric => metric) + // find column names + .map(metric => metric.column ? metric.column.column_name : metric.column_name || metric) + .filter(name => name) + .every(name => columnNames.has(name)); +} + // adhoc metrics are stored as dictionaries in URL params. We convert them back into the // AdhocMetric class for typechecking, consistency and instance method access. function coerceAdhocMetrics(value) { @@ -135,14 +150,22 @@ export default class MetricsControl extends React.PureComponent { } componentWillReceiveProps(nextProps) { + const { value } = this.props; if ( - isEqual(this.props.columns) !== isEqual(nextProps.columns) || - isEqual(this.props.savedMetrics) !== isEqual(nextProps.savedMetrics) + !isEqual(this.props.columns, nextProps.columns) || + !isEqual(this.props.savedMetrics, nextProps.savedMetrics) ) { + this.setState({ options: this.optionsForSelect(nextProps) }); - this.props.onChange([]); + + // Remove metrics if selected value no longer a column + const containsAllMetrics = columnsContainAllMetrics(value, nextProps); + + if (!containsAllMetrics) { + this.props.onChange([]); + } } - if (this.props.value !== nextProps.value) { + if (value !== nextProps.value) { this.setState({ value: coerceAdhocMetrics(nextProps.value) }); } } diff --git a/superset/assets/src/explore/controlPanels/extraOverrides.js b/superset/assets/src/explore/controlPanels/extraOverrides.js new file mode 100644 index 0000000000000..c4b31829ff95c --- /dev/null +++ b/superset/assets/src/explore/controlPanels/extraOverrides.js @@ -0,0 +1,22 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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. + */ +// For individual deployments to add custom overrides +export default function extraOverrides(controlPanelConfigs) { + return controlPanelConfigs; +} diff --git a/superset/assets/src/explore/controlPanels/index.js b/superset/assets/src/explore/controlPanels/index.js index 7034e5157ff0e..221c5cf29b25f 100644 --- a/superset/assets/src/explore/controlPanels/index.js +++ b/superset/assets/src/explore/controlPanels/index.js @@ -22,6 +22,7 @@ */ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import * as sections from './sections'; +import extraOverrides from './extraOverrides'; import Area from './Area'; import Bar from './Bar'; @@ -72,7 +73,7 @@ import DeckPolygon from './DeckPolygon'; import DeckScatter from './DeckScatter'; import DeckScreengrid from './DeckScreengrid'; -export const controlPanelConfigs = { +export const controlPanelConfigs = extraOverrides({ area: Area, bar: Bar, big_number: BigNumber, @@ -122,7 +123,7 @@ export const controlPanelConfigs = { deck_scatter: DeckScatter, deck_screengrid: DeckScreengrid, -}; +}); export default controlPanelConfigs; From 7bc3006b8d092a14c6a3781bd4edea80e59aa50e Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Fri, 15 Mar 2019 09:19:30 -0700 Subject: [PATCH 20/86] [sqlparse] Fixing table name extraction for ill-defined query (#7029) (cherry picked from commit 07c340cf8203f13222f16efad1e55e202deb1865) --- superset/sql_parse.py | 6 +++--- tests/sql_parse_tests.py | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/superset/sql_parse.py b/superset/sql_parse.py index b653c88a2a001..8076f09c7470c 100644 --- a/superset/sql_parse.py +++ b/superset/sql_parse.py @@ -76,7 +76,7 @@ def get_statements(self): @staticmethod def __get_full_name(identifier): - if len(identifier.tokens) > 1 and identifier.tokens[1].value == '.': + if len(identifier.tokens) > 2 and identifier.tokens[1].value == '.': return '{}.{}'.format(identifier.tokens[0].value, identifier.tokens[2].value) return identifier.get_real_name() @@ -89,8 +89,8 @@ def __process_identifier(self, identifier): # exclude subselects if '(' not in str(identifier): table_name = self.__get_full_name(identifier) - if not table_name.startswith(CTE_PREFIX): - self._table_names.add(self.__get_full_name(identifier)) + if table_name and not table_name.startswith(CTE_PREFIX): + self._table_names.add(table_name) return # store aliases diff --git a/tests/sql_parse_tests.py b/tests/sql_parse_tests.py index e821fce25455a..56959397fa9c5 100644 --- a/tests/sql_parse_tests.py +++ b/tests/sql_parse_tests.py @@ -47,6 +47,11 @@ def test_simple_select(self): {'schemaname.tbname'}, self.extract_tables('SELECT * FROM schemaname.tbname')) + # Ill-defined schema/table. + self.assertEquals( + set(), + self.extract_tables('SELECT * FROM schemaname.')) + # quotes query = 'SELECT field1, field2 FROM tb_name' self.assertEquals({'tb_name'}, self.extract_tables(query)) From 4ffc1b3b3602231d86a4963bc0fa20c3aabf15a6 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Wed, 20 Mar 2019 17:14:15 -0700 Subject: [PATCH 21/86] [missing values] Removing replacing missing values (#4905) (cherry picked from commit 61add606ca16a6ba981ccde864b121f5464b697a) --- superset/assets/backendSync.json | 3 +- superset/assets/src/explore/controls.jsx | 2 +- superset/common/query_context.py | 23 ----- superset/viz.py | 102 ++++++++++------------- tests/core_tests.py | 10 --- tests/viz_tests.py | 12 --- 6 files changed, 46 insertions(+), 106 deletions(-) diff --git a/superset/assets/backendSync.json b/superset/assets/backendSync.json index d334778d68d91..97eb5d22a8ecb 100644 --- a/superset/assets/backendSync.json +++ b/superset/assets/backendSync.json @@ -3783,4 +3783,5 @@ "default": false } } -} \ No newline at end of file +} + diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index d50db9c65c19f..845dd1dc0592e 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -1350,7 +1350,7 @@ export const controls = { 'mean', 'min', 'max', - 'stdev', + 'std', 'var', ]), default: 'sum', diff --git a/superset/common/query_context.py b/superset/common/query_context.py index 5053372412509..f989dad7f82be 100644 --- a/superset/common/query_context.py +++ b/superset/common/query_context.py @@ -41,7 +41,6 @@ class QueryContext: to retrieve the data payload for a given viz. """ - default_fillna = 0 cache_type = 'df' enforce_numerical_metrics = True @@ -103,7 +102,6 @@ def get_query_result(self, query_object): self.df_metrics_to_num(df, query_object) df.replace([np.inf, -np.inf], np.nan) - df = self.handle_nulls(df) return { 'query': result.query, 'status': result.status, @@ -118,27 +116,6 @@ def df_metrics_to_num(self, df, query_object): if dtype.type == np.object_ and col in metrics: df[col] = pd.to_numeric(df[col], errors='coerce') - def handle_nulls(self, df): - fillna = self.get_fillna_for_columns(df.columns) - return df.fillna(fillna) - - def get_fillna_for_col(self, col): - """Returns the value to use as filler for a specific Column.type""" - if col and col.is_string: - return ' NULL' - return self.default_fillna - - def get_fillna_for_columns(self, columns=None): - """Returns a dict or scalar that can be passed to DataFrame.fillna""" - if columns is None: - return self.default_fillna - columns_dict = {col.column_name: col for col in self.datasource.columns} - fillna = { - c: self.get_fillna_for_col(columns_dict.get(c)) - for c in columns - } - return fillna - def get_data(self, df): return df.to_dict(orient='records') diff --git a/superset/viz.py b/superset/viz.py index ef7ee456ebe11..1a93adbfbe2b8 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -75,7 +75,6 @@ class BaseViz(object): verbose_name = 'Base Viz' credits = '' is_timeseries = False - default_fillna = 0 cache_type = 'df' enforce_numerical_metrics = True @@ -164,28 +163,6 @@ def run_extra_queries(self): """ pass - def handle_nulls(self, df): - fillna = self.get_fillna_for_columns(df.columns) - return df.fillna(fillna) - - def get_fillna_for_col(self, col): - """Returns the value to use as filler for a specific Column.type""" - if col: - if col.is_string: - return ' NULL' - return self.default_fillna - - def get_fillna_for_columns(self, columns=None): - """Returns a dict or scalar that can be passed to DataFrame.fillna""" - if columns is None: - return self.default_fillna - columns_dict = {col.column_name: col for col in self.datasource.columns} - fillna = { - c: self.get_fillna_for_col(columns_dict.get(c)) - for c in columns - } - return fillna - def get_samples(self): query_obj = self.query_obj() query_obj.update({ @@ -254,8 +231,7 @@ def get_df(self, query_obj=None): if self.enforce_numerical_metrics: self.df_metrics_to_num(df) - df.replace([np.inf, -np.inf], np.nan) - df = self.handle_nulls(df) + df.replace([np.inf, -np.inf], np.nan, inplace=True) return df def df_metrics_to_num(self, df): @@ -653,7 +629,9 @@ def get_data(self, df): pt = df.pivot_table( index=DTTM_ALIAS, columns=columns, - values=values) + values=values, + dropna=False, + ) pt.index = pt.index.map(str) pt = pt.sort_index() return dict( @@ -696,12 +674,20 @@ def get_data(self, df): self.form_data.get('granularity') == 'all' and DTTM_ALIAS in df): del df[DTTM_ALIAS] + + aggfunc = self.form_data.get('pandas_aggfunc') + + # Ensure that Pandas's sum function mimics that of SQL. + if aggfunc == 'sum': + aggfunc = lambda x: x.sum(min_count=1) # noqa: E731 + df = df.pivot_table( index=self.form_data.get('groupby'), columns=self.form_data.get('columns'), values=[utils.get_metric_name(m) for m in self.form_data.get('metrics')], - aggfunc=self.form_data.get('pandas_aggfunc'), + aggfunc=aggfunc, margins=self.form_data.get('pivot_margins'), + dropna=False, ) # Display metrics side by side with each column if self.form_data.get('combine_metric'): @@ -709,7 +695,7 @@ def get_data(self, df): return dict( columns=list(df.columns), html=df.to_html( - na_rep='', + na_rep='null', classes=( 'dataframe table table-striped table-bordered ' 'table-condensed table-hover').split(' ')), @@ -877,7 +863,7 @@ def to_series(self, df, classed='', title_suffix=''): index_value = label_sep.join(index_value) boxes = defaultdict(dict) for (label, key), value in row.items(): - if key == 'median': + if key == 'nanmedian': key = 'Q2' boxes[label][key] = value for label, box in boxes.items(): @@ -894,28 +880,24 @@ def to_series(self, df, classed='', title_suffix=''): def get_data(self, df): form_data = self.form_data - df = df.fillna(0) # conform to NVD3 names def Q1(series): # need to be named functions - can't use lambdas - return np.percentile(series, 25) + return np.nanpercentile(series, 25) def Q3(series): - return np.percentile(series, 75) + return np.nanpercentile(series, 75) whisker_type = form_data.get('whisker_options') if whisker_type == 'Tukey': def whisker_high(series): upper_outer_lim = Q3(series) + 1.5 * (Q3(series) - Q1(series)) - series = series[series <= upper_outer_lim] - return series[np.abs(series - upper_outer_lim).argmin()] + return series[series <= upper_outer_lim].max() def whisker_low(series): lower_outer_lim = Q1(series) - 1.5 * (Q3(series) - Q1(series)) - # find the closest value above the lower outer limit - series = series[series >= lower_outer_lim] - return series[np.abs(series - lower_outer_lim).argmin()] + return series[series >= lower_outer_lim].min() elif whisker_type == 'Min/max (no outliers)': @@ -929,10 +911,10 @@ def whisker_low(series): low, high = whisker_type.replace(' percentiles', '').split('/') def whisker_high(series): - return np.percentile(series, int(high)) + return np.nanpercentile(series, int(high)) def whisker_low(series): - return np.percentile(series, int(low)) + return np.nanpercentile(series, int(low)) else: raise ValueError('Unknown whisker type: {}'.format(whisker_type)) @@ -943,7 +925,7 @@ def outliers(series): # pandas sometimes doesn't like getting lists back here return set(above.tolist() + below.tolist()) - aggregate = [Q1, np.median, Q3, whisker_high, whisker_low, outliers] + aggregate = [Q1, np.nanmedian, Q3, whisker_high, whisker_low, outliers] df = df.groupby(form_data.get('groupby')).agg(aggregate) chart_data = self.to_series(df) return chart_data @@ -1034,7 +1016,6 @@ def as_floats(field): return d def get_data(self, df): - df = df.fillna(0) df['metric'] = df[[utils.get_metric_name(self.metric)]] values = df['metric'].values return { @@ -1152,7 +1133,6 @@ def to_series(self, df, classed='', title_suffix=''): def process_data(self, df, aggregate=False): fd = self.form_data - df = df.fillna(0) if fd.get('granularity') == 'all': raise Exception(_('Pick a time granularity for your time series')) @@ -1160,14 +1140,18 @@ def process_data(self, df, aggregate=False): df = df.pivot_table( index=DTTM_ALIAS, columns=fd.get('groupby'), - values=self.metric_labels) + values=self.metric_labels, + dropna=False, + ) else: df = df.pivot_table( index=DTTM_ALIAS, columns=fd.get('groupby'), values=self.metric_labels, fill_value=0, - aggfunc=sum) + aggfunc=sum, + dropna=False, + ) fm = fd.get('resample_fillmethod') if not fm: @@ -1176,8 +1160,6 @@ def process_data(self, df, aggregate=False): rule = fd.get('resample_rule') if how and rule: df = df.resample(rule, how=how, fill_method=fm) - if not fm: - df = df.fillna(0) if self.sort_series: dfs = df.sum() @@ -1241,7 +1223,6 @@ def get_data(self, df): fd = self.form_data comparison_type = fd.get('comparison_type') or 'values' df = self.process_data(df) - if comparison_type == 'values': chart_data = self.to_series(df) for i, (label, df2) in enumerate(self._extra_chart_data): @@ -1368,7 +1349,6 @@ def to_series(self, df, classed=''): def get_data(self, df): fd = self.form_data - df = df.fillna(0) if self.form_data.get('granularity') == 'all': raise Exception(_('Pick a time granularity for your time series')) @@ -1377,7 +1357,9 @@ def get_data(self, df): metric_2 = utils.get_metric_name(fd.get('metric_2')) df = df.pivot_table( index=DTTM_ALIAS, - values=[metric, metric_2]) + values=[metric, metric_2], + dropna=False, + ) chart_data = self.to_series(df) return chart_data @@ -1425,7 +1407,9 @@ def get_data(self, df): df = df.pivot_table( index=DTTM_ALIAS, columns='series', - values=utils.get_metric_name(fd.get('metric'))) + values=utils.get_metric_name(fd.get('metric')), + dropna=False, + ) chart_data = self.to_series(df) for serie in chart_data: serie['rank'] = rank_lookup[serie['key']] @@ -1462,7 +1446,9 @@ def get_data(self, df): metric = self.metric_labels[0] df = df.pivot_table( index=self.groupby, - values=[metric]) + values=[metric], + dropna=False, + ) df.sort_values(by=metric, ascending=False, inplace=True) df = df.reset_index() df.columns = ['x', 'y'] @@ -1549,9 +1535,10 @@ def get_data(self, df): pt = df.pivot_table( index=self.groupby, columns=columns, - values=metrics) + values=metrics, + dropna=False, + ) if fd.get('contribution'): - pt = pt.fillna(0) pt = pt.T pt = (pt / pt.sum()).T pt = pt.reindex(row.index) @@ -2117,9 +2104,6 @@ class BaseDeckGLViz(BaseViz): credits = 'deck.gl' spatial_control_keys = [] - def handle_nulls(self, df): - return df - def get_metrics(self): self.metric = self.form_data.get('size') return [self.metric] if self.metric else [] @@ -2572,11 +2556,11 @@ def get_data(self, df): fd = self.form_data groups = fd.get('groupby') metrics = fd.get('metrics') - df.fillna(0) df = df.pivot_table( index=DTTM_ALIAS, columns=groups, - values=metrics) + values=metrics, + ) cols = [] # Be rid of falsey keys for col in df.columns: @@ -2699,7 +2683,7 @@ def levels_for_time(self, groups, df): for i in range(0, len(groups) + 1): self.form_data['groupby'] = groups[:i] df_drop = df.drop(groups[i:], 1) - procs[i] = self.process_data(df_drop, aggregate=True).fillna(0) + procs[i] = self.process_data(df_drop, aggregate=True) self.form_data['groupby'] = groups return procs diff --git a/tests/core_tests.py b/tests/core_tests.py index 326025e1b2f13..14a6ed80f2073 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -622,16 +622,6 @@ def test_slice_query_endpoint(self): assert 'language' in resp self.logout() - def test_viz_get_fillna_for_columns(self): - slc = self.get_slice('Girls', db.session) - q = slc.viz.query_obj() - results = slc.viz.datasource.query(q) - fillna_columns = slc.viz.get_fillna_for_columns(results.df.columns) - self.assertDictEqual( - fillna_columns, - {'name': ' NULL', 'sum__num': 0}, - ) - def test_import_csv(self): self.login(username='admin') filename = 'testCSV.csv' diff --git a/tests/viz_tests.py b/tests/viz_tests.py index b23cacda0a337..b84c66184e7ba 100644 --- a/tests/viz_tests.py +++ b/tests/viz_tests.py @@ -84,18 +84,6 @@ def test_process_metrics(self): self.assertEqual(test_viz.metric_labels, expect_metric_labels) self.assertEqual(test_viz.all_metrics, expect_metric_labels) - def test_get_fillna_returns_default_on_null_columns(self): - form_data = { - 'viz_type': 'table', - 'token': '12345', - } - datasource = self.get_datasource_mock() - test_viz = viz.BaseViz(datasource, form_data) - self.assertEqual( - test_viz.default_fillna, - test_viz.get_fillna_for_columns(), - ) - def test_get_df_returns_empty_df(self): form_data = {'dummy': 123} query_obj = {'granularity': 'day'} From dfc0010b22aa44b953d1d59e0b5ca416a351fa50 Mon Sep 17 00:00:00 2001 From: Enrico Berti Date: Thu, 21 Mar 2019 23:13:42 +0100 Subject: [PATCH 22/86] [SQL Lab] Improved query and results tabs rendering reliability (#7082) closes #7080 (cherry picked from commit 9b58e9f4920ef424e5b545dcbb4726e22bed5982) --- superset/assets/src/SqlLab/components/SouthPane.jsx | 1 + superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx | 1 + 2 files changed, 2 insertions(+) diff --git a/superset/assets/src/SqlLab/components/SouthPane.jsx b/superset/assets/src/SqlLab/components/SouthPane.jsx index e3e51e204fc83..441104c4d745a 100644 --- a/superset/assets/src/SqlLab/components/SouthPane.jsx +++ b/superset/assets/src/SqlLab/components/SouthPane.jsx @@ -118,6 +118,7 @@ export class SouthPane extends React.PureComponent {
Date: Wed, 20 Mar 2019 21:41:33 -0700 Subject: [PATCH 23/86] Fix filter_box migration PR #6523 (#7066) * Fix filter_box migration PR #6523 * Fix druid-related bug (cherry picked from commit b210742ad24d01ca05bc58ca3342c90e301fe073) --- superset/connectors/druid/models.py | 4 +- .../versions/fb13d49b72f9_better_filters.py | 40 ++++++++++-------- tests/migration_tests.py | 42 +++++++++++++++++++ 3 files changed, 67 insertions(+), 19 deletions(-) create mode 100644 tests/migration_tests.py diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 577679092c09b..3b22ade1df366 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -1144,7 +1144,9 @@ def run_query( # noqa / druid pre_qry['aggregations'] = aggs_dict pre_qry['post_aggregations'] = post_aggs_dict else: - order_by = list(qry['aggregations'].keys())[0] + agg_keys = qry['aggregations'].keys() + order_by = list(agg_keys)[0] if agg_keys else None + # Limit on the number of timeseries, doing a two-phases query pre_qry['granularity'] = 'all' pre_qry['threshold'] = min(row_limit, diff --git a/superset/migrations/versions/fb13d49b72f9_better_filters.py b/superset/migrations/versions/fb13d49b72f9_better_filters.py index 956766d060b79..cac42c3dc6147 100644 --- a/superset/migrations/versions/fb13d49b72f9_better_filters.py +++ b/superset/migrations/versions/fb13d49b72f9_better_filters.py @@ -46,6 +46,27 @@ class Slice(Base): slice_name = Column(String(250)) +def upgrade_slice(slc): + params = json.loads(slc.params) + logging.info(f'Upgrading {slc.slice_name}') + cols = params.get('groupby') + metric = params.get('metric') + if cols: + flts = [{ + 'column': col, + 'metric': metric, + 'asc': False, + 'clearable': True, + 'multiple': True, + } for col in cols] + params['filter_configs'] = flts + if 'groupby' in params: + del params['groupby'] + if 'metric' in params: + del params['metric'] + slc.params = json.dumps(params, sort_keys=True) + + def upgrade(): bind = op.get_bind() session = db.Session(bind=bind) @@ -53,24 +74,7 @@ def upgrade(): filter_box_slices = session.query(Slice).filter_by(viz_type='filter_box') for slc in filter_box_slices.all(): try: - params = json.loads(slc.params) - logging.info(f'Upgrading {slc.slice_name}') - cols = params.get('groupby') - metrics = params.get('metrics') - if cols: - flts = [{ - 'column': col, - 'metric': metrics[0] if metrics else None, - 'asc': False, - 'clearable': True, - 'multiple': True, - } for col in cols] - params['filter_configs'] = flts - if 'groupby' in params: - del params['groupby'] - if 'metrics' in params: - del params['metrics'] - slc.params = json.dumps(params, sort_keys=True) + upgrade_slice(slc) except Exception as e: logging.exception(e) diff --git a/tests/migration_tests.py b/tests/migration_tests.py new file mode 100644 index 0000000000000..530e0060361a3 --- /dev/null +++ b/tests/migration_tests.py @@ -0,0 +1,42 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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 json + +from superset.migrations.versions.fb13d49b72f9_better_filters import ( + Slice, upgrade_slice, +) +from .base_tests import SupersetTestCase + + +class MigrationTestCase(SupersetTestCase): + + def test_upgrade_slice(self): + slc = Slice( + slice_name='FOO', + viz_type='filter_box', + params=json.dumps(dict( + metric='foo', + groupby=['bar'], + )), + ) + upgrade_slice(slc) + params = json.loads(slc.params) + self.assertNotIn('metric', params) + self.assertIn('filter_configs', params) + + cfg = params['filter_configs'][0] + self.assertEquals(cfg.get('metric'), 'foo') From 959199d354d82442c1d5f2b1be5700ff8539567e Mon Sep 17 00:00:00 2001 From: Christine Chambers Date: Mon, 25 Mar 2019 15:19:43 -0700 Subject: [PATCH 24/86] SQL editor layout makeover (#7102) This PR includes the following layout and css tweaks: - Using flex to layout the north and south sub panes of query pane so resizing works properly in both Chrome and Firefox - Removal of necessary wrapper divs and tweaking of css in sql lab so we can scroll to the bottom of both the table list and the results pane - Make sql lab's content not overflow vertically and layout the query result area to eliminate double scroll bars - css tweaks on the basic.html page so the loading animation appears in the center of the page across the board (cherry picked from commit 71f1bbd2ec59b99d6ba6d9a4a2f9cfceaf922b80) --- .../assets/cypress/integration/sqllab/tabs.js | 2 +- superset/assets/package-lock.json | 43 ++--- superset/assets/src/SqlLab/components/App.jsx | 16 +- .../src/SqlLab/components/ResultSet.jsx | 4 +- .../src/SqlLab/components/SouthPane.jsx | 13 +- .../src/SqlLab/components/SqlEditor.jsx | 72 +++++---- .../SqlLab/components/SqlEditorLeftBar.jsx | 16 +- .../SqlLab/components/TabbedSqlEditors.jsx | 108 ++++++------- superset/assets/src/SqlLab/main.less | 152 +++++++++++------- .../FilterableTable/FilterableTable.jsx | 19 ++- .../FilterableTable/FilterableTableStyles.css | 3 - .../assets/src/components/TableSelector.css | 2 +- .../src/dashboard/stylesheets/dashboard.less | 12 +- .../stylesheets/less/cosmo/bootswatch.less | 36 ++--- superset/assets/stylesheets/less/index.less | 19 +++ superset/assets/stylesheets/superset.less | 15 +- superset/templates/superset/basic.html | 4 +- 17 files changed, 286 insertions(+), 250 deletions(-) diff --git a/superset/assets/cypress/integration/sqllab/tabs.js b/superset/assets/cypress/integration/sqllab/tabs.js index 17026ced8e8cd..0d9731ef982b2 100644 --- a/superset/assets/cypress/integration/sqllab/tabs.js +++ b/superset/assets/cypress/integration/sqllab/tabs.js @@ -42,7 +42,7 @@ export default () => { const initialTabCount = tabListA.length; // open the tab dropdown to remove - cy.get('#a11y-query-editor-tabs > ul > li:first button').click(); + cy.get('#a11y-query-editor-tabs > ul > li:first button:nth-child(2)').click(); // first item is close cy.get('#a11y-query-editor-tabs > ul > li:first ul li a') diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json index 9eae0292ecd16..48beb9cdb4449 100644 --- a/superset/assets/package-lock.json +++ b/superset/assets/package-lock.json @@ -8790,8 +8790,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -8812,14 +8811,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -8834,20 +8831,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -8964,8 +8958,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -8977,7 +8970,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -8992,7 +8984,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -9000,14 +8991,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -9026,7 +9015,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -9107,8 +9095,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -9120,7 +9107,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -9206,8 +9192,7 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -9243,7 +9228,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -9263,7 +9247,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -9307,14 +9290,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, @@ -16670,7 +16651,7 @@ }, "minimist": { "version": "0.0.8", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" }, "mjolnir.js": { diff --git a/superset/assets/src/SqlLab/components/App.jsx b/superset/assets/src/SqlLab/components/App.jsx index 25c61deb64526..e4e516aeac90d 100644 --- a/superset/assets/src/SqlLab/components/App.jsx +++ b/superset/assets/src/SqlLab/components/App.jsx @@ -71,26 +71,18 @@ class App extends React.PureComponent { render() { let content; if (this.state.hash) { - content = ( -
-
-
- -
-
-
- ); + content = ; } else { content = ( -
+ -
+ ); } return (
-
{content}
+ {content}
); diff --git a/superset/assets/src/SqlLab/components/ResultSet.jsx b/superset/assets/src/SqlLab/components/ResultSet.jsx index 1b382e69b5056..641cae5f2223e 100644 --- a/superset/assets/src/SqlLab/components/ResultSet.jsx +++ b/superset/assets/src/SqlLab/components/ResultSet.jsx @@ -203,7 +203,7 @@ export default class ResultSet extends React.PureComponent { } if (data && data.length > 0) { return ( -
+ {this.renderControls.bind(this)()} {sql} -
+ ); } else if (data && data.length === 0) { return The query returned no data; diff --git a/superset/assets/src/SqlLab/components/SouthPane.jsx b/superset/assets/src/SqlLab/components/SouthPane.jsx index 441104c4d745a..c3dd57ba7b4b9 100644 --- a/superset/assets/src/SqlLab/components/SouthPane.jsx +++ b/superset/assets/src/SqlLab/components/SouthPane.jsx @@ -29,6 +29,8 @@ import QueryHistory from './QueryHistory'; import ResultSet from './ResultSet'; import { STATUS_OPTIONS, STATE_BSSTYLE_MAP } from '../constants'; +const TAB_HEIGHT = 44; + /* editorQueries are queries executed by users passed from SqlEditor component dataPrebiewQueries are all queries executed for preview of table data (from SqlEditorLeft) @@ -76,7 +78,7 @@ export class SouthPane extends React.PureComponent { { STATUS_OPTIONS.offline } ); } - const innerTabHeight = this.state.height - 55; + const innerTabContentHeight = this.state.height - TAB_HEIGHT; let latestQuery; const props = this.props; if (props.editorQueries.length > 0) { @@ -90,7 +92,7 @@ export class SouthPane extends React.PureComponent { search query={latestQuery} actions={props.actions} - height={innerTabHeight} + height={innerTabContentHeight} database={this.props.databases[latestQuery.dbId]} /> ); @@ -109,7 +111,7 @@ export class SouthPane extends React.PureComponent { csv={false} actions={props.actions} cache - height={innerTabHeight} + height={innerTabContentHeight} /> )); @@ -119,6 +121,7 @@ export class SouthPane extends React.PureComponent { -
- -
+ {dataPreviewTabs}
diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx index cf142b2d88ce5..fdb7d45ee30ea 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -44,8 +44,10 @@ import AceEditorWrapper from './AceEditorWrapper'; import { STATE_BSSTYLE_MAP } from '../constants'; import RunQueryActionButton from './RunQueryActionButton'; +const SQL_EDITOR_PADDING = 10; const SQL_TOOLBAR_HEIGHT = 51; const GUTTER_HEIGHT = 5; +const GUTTER_MARGIN = 3; const INITIAL_NORTH_PERCENT = 30; const INITIAL_SOUTH_PERCENT = 70; @@ -81,6 +83,7 @@ class SqlEditor extends React.PureComponent { this.sqlEditorRef = React.createRef(); this.northPaneRef = React.createRef(); + this.elementStyle = this.elementStyle.bind(this); this.onResizeStart = this.onResizeStart.bind(this); this.onResizeEnd = this.onResizeEnd.bind(this); this.runQuery = this.runQuery.bind(this); @@ -124,14 +127,16 @@ class SqlEditor extends React.PureComponent { } // One layer of abstraction for easy spying in unit tests getSqlEditorHeight() { - return this.sqlEditorRef.current ? this.sqlEditorRef.current.clientHeight : 0; + return this.sqlEditorRef.current ? + (this.sqlEditorRef.current.clientHeight - SQL_EDITOR_PADDING * 2) : 0; } // Return the heights for the ace editor and the south pane as an object // given the height of the sql editor, north pane percent and south pane percent. getAceEditorAndSouthPaneHeights(height, northPercent, southPercent) { return { - aceEditorHeight: height * northPercent / 100 - SQL_TOOLBAR_HEIGHT - GUTTER_HEIGHT / 2, - southPaneHeight: height * southPercent / 100, + aceEditorHeight: height * northPercent / 100 - (GUTTER_HEIGHT / 2 + GUTTER_MARGIN) + - SQL_TOOLBAR_HEIGHT, + southPaneHeight: height * southPercent / 100 - (GUTTER_HEIGHT / 2 + GUTTER_MARGIN), }; } getHotkeyConfig() { @@ -174,6 +179,11 @@ class SqlEditor extends React.PureComponent { setQueryLimit(queryLimit) { this.props.actions.queryEditorSetQueryLimit(this.props.queryEditor, queryLimit); } + elementStyle(dimension, elementSize, gutterSize) { + return { + [dimension]: `calc(${elementSize}% - ${gutterSize + GUTTER_MARGIN}px)`, + }; + } runQuery() { if (this.props.database) { this.startQuery(this.props.database.allow_run_async); @@ -212,36 +222,36 @@ class SqlEditor extends React.PureComponent { const { aceEditorHeight, southPaneHeight } = this.getAceEditorAndSouthPaneHeights( this.state.height, INITIAL_NORTH_PERCENT, INITIAL_SOUTH_PERCENT); return ( -
- -
- - {this.renderEditorBottomBar(hotkeys)} -
- +
+ - -
+ {this.renderEditorBottomBar(hotkeys)} +
+ + ); } renderEditorBottomBar(hotkeys) { diff --git a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx index 0de18b0a6baeb..9d0796cd8ce74 100644 --- a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx @@ -105,7 +105,7 @@ export default class SqlEditorLeftBar extends React.PureComponent { const tableMetaDataHeight = this.props.height - 130; // 130 is the height of the selects above const qe = this.props.queryEditor; return ( -
+
-
-
-
-
- {this.props.tables.map(table => ( - - ))} -
+
+
+
+ {this.props.tables.map(table => ( + + ))}
{shouldShowReset && diff --git a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx index c287b700e7a71..a8516f174387b 100644 --- a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx +++ b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx @@ -18,7 +18,7 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { DropdownButton, MenuItem, Tab, Tabs } from 'react-bootstrap'; +import { MenuItem, SplitButton, Tab, Tabs } from 'react-bootstrap'; import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import URI from 'urijs'; @@ -196,64 +196,62 @@ class TabbedSqlEditors extends React.PureComponent { } const state = latestQuery ? latestQuery.state : ''; - const tabTitle = ( -
+ const title = ( + this.removeQueryEditor(qe)} tabState={state} />{' '} {qe.title}{' '} - - this.removeQueryEditor(qe)}> -
- -
- {t('Close tab')} -
- this.renameTab(qe)}> -
- -
- {t('Rename tab')} -
- -
- -
- {this.state.hideLeftBar ? t('Expand tool bar') : t('Hide tool bar')} -
- this.removeAllOtherQueryEditors(qe)}> -
- -
- {t('Close all other tabs')} -
-
-
+ + ); + const tabTitle = ( + + this.removeQueryEditor(qe)}> +
+ +
+ {t('Close tab')} +
+ this.renameTab(qe)}> +
+ +
+ {t('Rename tab')} +
+ +
+ +
+ {this.state.hideLeftBar ? t('Expand tool bar') : t('Hide tool bar')} +
+ this.removeAllOtherQueryEditors(qe)}> +
+ +
+ {t('Close all other tabs')} +
+
); return ( -
-
- {isSelected && ( - xt.queryEditorId === qe.id)} - queryEditor={qe} - editorQueries={this.state.queriesArray} - dataPreviewQueries={this.state.dataPreviewQueries} - latestQuery={latestQuery} - database={database} - actions={this.props.actions} - hideLeftBar={this.state.hideLeftBar} - defaultQueryLimit={this.props.defaultQueryLimit} - maxRow={this.props.maxRow} - saveQueryWarning={this.props.saveQueryWarning} - /> - )} -
-
+ {isSelected && ( + xt.queryEditorId === qe.id)} + queryEditor={qe} + editorQueries={this.state.queriesArray} + dataPreviewQueries={this.state.dataPreviewQueries} + latestQuery={latestQuery} + database={database} + actions={this.props.actions} + hideLeftBar={this.state.hideLeftBar} + defaultQueryLimit={this.props.defaultQueryLimit} + maxRow={this.props.maxRow} + saveQueryWarning={this.props.saveQueryWarning} + /> + )}
); }); @@ -264,6 +262,7 @@ class TabbedSqlEditors extends React.PureComponent { activeKey={this.props.tabHistory[this.props.tabHistory.length - 1]} onSelect={this.handleSelect.bind(this)} id="a11y-query-editor-tabs" + className="SqlEditorTabs" > {editors}  
} + className="addEditorTab" eventKey="add_tab" disabled={this.props.offline} /> diff --git a/superset/assets/src/SqlLab/main.less b/superset/assets/src/SqlLab/main.less index 6c35ed9c4fda6..89e5d11b9f0d0 100644 --- a/superset/assets/src/SqlLab/main.less +++ b/superset/assets/src/SqlLab/main.less @@ -17,9 +17,11 @@ * under the License. */ @import "../../stylesheets/less/cosmo/variables.less"; + body { overflow: hidden; } + .inlineBlock { display: inline-block; } @@ -32,10 +34,6 @@ body { .nopadding { padding: 0px; } -.panel.nopadding .panel-body { - padding: 0px; -} - .loading { width: 50px; margin-top: 15px; @@ -46,27 +44,19 @@ body { width: 100%; height: 100%; } -.SqlEditor .header { - padding-top: 5px; - padding-bottom: 5px; -} - -.scrollbar-container { - position: relative; - overflow: hidden; - width: 100%; - height: 100%; -} -.scrollbar-content { - position: absolute; - top: 0px; - left: 0px; - right: 0px; - bottom: 0px; - overflow: auto; - margin-right: 0px; - margin-bottom: 0px; +.tab-content { + height: 100%; + position: relative; + background-color: #fff; + + > .tab-pane { + position: absolute; + top: 0; + right: 0; + bottom: 0; + left: 0; + } } .Workspace .btn-sm { @@ -82,11 +72,6 @@ div.Workspace { height: 100%; margin: 0px; } -.SqlEditor .clock { - background-color: orange; - padding: 5px; -} - .padded { padding: 10px; } @@ -194,10 +179,19 @@ div.Workspace { background-color: transparent !important; } +#a11y-query-editor-tabs { + height: 100%; + display: flex; + flex-direction: column; +} + .SqlLab { - .tab-content { - height: 100%; - } + position: absolute; + top: 0px; + right: 0px; + bottom: 0px; + left: 0px; + padding: 0 10px; #brace-editor { height: calc(100% - 51px); @@ -212,16 +206,46 @@ div.Workspace { } } +.SqlEditorTabs li { + a:focus { + outline: 0; + } + + .ddbtn-tab { + font-size: inherit; + font-weight: bold; + + &:active { + background: none; + } + } + + .dropdown-toggle { + padding-top: 2px; + } +} + .SqlEditor { display: flex; flex-direction: row; height: 100%; + padding: 10px; + + .clock { + background-color: orange; + padding: 5px; + } .schemaPane { - flex-grow: 1; + flex: 0 0 300px; transition: all .3s ease-in-out; } + .queryPane { + flex: 1 1 auto; + padding-left: 10px; + } + .schemaPane-enter-done, .schemaPane-exit { transform: translateX(0); } @@ -236,12 +260,6 @@ div.Workspace { overflow: hidden; } - .queryPane { - flex-grow: 8; - position: relative; - margin-left: 15px; - } - .schemaPane-exit-done + .queryPane { margin-left: 0; } @@ -258,6 +276,22 @@ div.Workspace { } } +.SqlEditorLeftBar { + height: 100%; + display: flex; + flex-direction: column; + + .divider { + border-bottom: 1px solid #f2f2f2; + margin: 15px 0; + } + + .scrollbar-container { + flex: 1 1 auto; + overflow: auto; + } +} + .popover{ max-width:400px; } @@ -276,8 +310,7 @@ div.tablePopover:hover { opacity: 1 !important; } .ResultSetControls { - padding-bottom: 3px; - padding-top: 3px; + padding: 8px 0; } .ace_editor { border: 1px solid #ccc; @@ -298,10 +331,6 @@ div.tablePopover:hover { background-color: #f4f4f4; } -.SouthPane .tab-content { - padding-top: 10px; -} - .TableElement { margin-right: 10px; } @@ -346,22 +375,29 @@ a.Link { max-width: 500px; } .SouthPane { - margin-top: 10px; - position: absolute; width: 100%; - overflow: auto; -} -.nav-tabs > li.active > a, -.nav-tabs > li.active > a:hover, -.nav-tabs > li.active > a:focus { - padding-bottom: 8px; -} -.nav-tabs .dropdown-toggle.btn .caret { - margin-top: -12px; + + .SouthPaneTabs { + height: 100%; + display: flex; + flex-direction: column; + } + .tab-pane { + overflow-y: auto; // scroll the query history pane + } } + .nav-tabs .ddbtn-tab { - margin-left: 5px; - padding-right: 0; + padding: 0; + border: none; + background: none; + + &:focus { + outline: 0; + } + &:active { + box-shadow: none; + } } .icon-container { display: inline-block; diff --git a/superset/assets/src/components/FilterableTable/FilterableTable.jsx b/superset/assets/src/components/FilterableTable/FilterableTable.jsx index 4b804b0ab7e09..ed593f5c07279 100644 --- a/superset/assets/src/components/FilterableTable/FilterableTable.jsx +++ b/superset/assets/src/components/FilterableTable/FilterableTable.jsx @@ -28,6 +28,8 @@ import { } from 'react-virtualized'; import { getTextWidth } from '../../modules/visUtils'; +const SCROLL_BAR_HEIGHT = 15; + const propTypes = { orderedColumnKeys: PropTypes.array.isRequired, data: PropTypes.array.isRequired, @@ -59,6 +61,7 @@ export default class FilterableTable extends PureComponent { this.totalTableWidth = props.orderedColumnKeys .map(key => this.widthsForColumnsByKey[key]) .reduce((curr, next) => curr + next); + this.totalTableHeight = props.height; this.state = { sortBy: null, @@ -89,9 +92,10 @@ export default class FilterableTable extends PureComponent { } fitTableToWidthIfNeeded() { - const containerWidth = this.container.getBoundingClientRect().width; - if (containerWidth > this.totalTableWidth) { - this.totalTableWidth = containerWidth - 2; // accommodates 1px border on container + const containerWidth = this.container.clientWidth; + if (this.totalTableWidth < containerWidth) { + // fit table width if content doesn't fill the width of the container + this.totalTableWidth = containerWidth; } this.setState({ fitted: true }); } @@ -174,6 +178,13 @@ export default class FilterableTable extends PureComponent { .update(list => sortDirection === SortDirection.DESC ? list.reverse() : list); } + let totalTableHeight = height; + if (this.container && this.totalTableWidth > this.container.clientWidth) { + // exclude the height of the horizontal scroll bar from the height of the table + // if the content overflows + totalTableHeight -= SCROLL_BAR_HEIGHT; + } + const rowGetter = ({ index }) => this.getDatum(sortedAndFilteredList, index); return (
a:focus { + outline: 0; + } + &-inverse { .badge { @@ -31,6 +36,11 @@ color: @brand-primary; } } + + b.caret { + display: inline-block; + padding: 0 5px 18px 5px; + } } .navbar-inverse { @@ -92,28 +102,6 @@ color: @gray-darker; } -.dropdown-toggle.btn .caret { - margin-left: 6px; - margin-top: -21px; - margin-right: 6px; -} - -.nav-tabs .dropdown-toggle.btn .caret { - margin-left: -12px; - margin-top: -10px; -} - -.navbar-nav .caret, -.panel-title .caret { - margin-left: 6px; - margin-top: -24px; - margin-right: 6px; -} - - - - - // Typography ================================================================= body { @@ -408,6 +396,10 @@ a.list-group-item { // Tabs ============================================================== +.nav-tabs > li > a { + border-top: 3px solid transparent; +} + .nav-tabs > li.active > a, .nav-tabs > li.active > a:hover, .nav-tabs > li.active > a:focus { background-color: #fff; font-weight: bold; diff --git a/superset/assets/stylesheets/less/index.less b/superset/assets/stylesheets/less/index.less index b9a455acd74c3..b1fc604323c0e 100644 --- a/superset/assets/stylesheets/less/index.less +++ b/superset/assets/stylesheets/less/index.less @@ -23,3 +23,22 @@ @import "./cosmo/bootswatch.less"; @stroke-primary: @brand-primary; + +body { + position: absolute; + top: 0; + right: 0; + bottom: 0; + left: 0; + display: flex; + flex-direction: column; +} + +header { + flex: 0 1 auto; +} + +#app { + flex: 1 1 auto; + position: relative; +} diff --git a/superset/assets/stylesheets/superset.less b/superset/assets/stylesheets/superset.less index ebd648b094860..587851ea00e8d 100644 --- a/superset/assets/stylesheets/superset.less +++ b/superset/assets/stylesheets/superset.less @@ -21,14 +21,6 @@ @datasource-sql-expression-width: 450px; -body { - margin: 0 !important; -} - -.caret { - border-top: 4px solid; -} - .emph { font-weight: bold !important; } @@ -413,6 +405,13 @@ table.table-no-hover tr:hover { display: inline; } +.form-actions-container button { + display: flex; + .caret { + margin: 0 8px; + } +} + .list-container .filter-action { margin: 10px 10px 0 10px; padding-bottom: 15px; diff --git a/superset/templates/superset/basic.html b/superset/templates/superset/basic.html index 28835d1cec613..6c1ff626c752c 100644 --- a/superset/templates/superset/basic.html +++ b/superset/templates/superset/basic.html @@ -69,8 +69,8 @@ {% endblock %} {% block body %} -
- +
+
{% endblock %} From b599855994e6ff407aa70200bbd505ccd86b9ce5 Mon Sep 17 00:00:00 2001 From: John Bodley Date: Wed, 20 Mar 2019 21:24:14 -0700 Subject: [PATCH 25/86] [forms] Fix handling of NULLs (cherry picked from commit e83a07d3dfda350cc44041cb6cbaec4510887902) --- superset/models/core.py | 16 ++++++++++++---- superset/views/core.py | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/superset/models/core.py b/superset/models/core.py index c8435c2c398d6..b848604a46d64 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -163,7 +163,7 @@ class Slice(Model, AuditMixinNullable, ImportMixin): 'viz_type', 'params', 'cache_timeout') def __repr__(self): - return self.slice_name + return self.slice_name or str(self.id) @property def cls_model(self): @@ -291,10 +291,14 @@ def explore_json_url(self): def edit_url(self): return '/chart/edit/{}'.format(self.id) + @property + def chart(self): + return self.slice_name or '' + @property def slice_link(self): url = self.slice_url - name = escape(self.slice_name) + name = escape(self.chart) return Markup(f'{name}') def get_viz(self, force=False): @@ -407,7 +411,7 @@ class Dashboard(Model, AuditMixinNullable, ImportMixin): 'description', 'css', 'slug') def __repr__(self): - return self.dashboard_title + return self.dashboard_title or str(self.id) @property def table_names(self): @@ -436,6 +440,10 @@ def url(self): def datasources(self): return {slc.datasource for slc in self.slices} + @property + def charts(self): + return [slc.chart for slc in self.slices] + @property def sqla_metadata(self): # pylint: disable=no-member @@ -443,7 +451,7 @@ def sqla_metadata(self): return metadata.reflect() def dashboard_link(self): - title = escape(self.dashboard_title) + title = escape(self.dashboard_title or '') return Markup(f'{title}') @property diff --git a/superset/views/core.py b/superset/views/core.py index 3ceec89f50577..f3b356db06928 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -595,7 +595,7 @@ class DashboardModelView(SupersetModelView, DeleteMixin): # noqa edit_columns = [ 'dashboard_title', 'slug', 'owners', 'position_json', 'css', 'json_metadata'] - show_columns = edit_columns + ['table_names', 'slices'] + show_columns = edit_columns + ['table_names', 'charts'] search_columns = ('dashboard_title', 'slug', 'owners') add_columns = edit_columns base_order = ('changed_on', 'desc') @@ -622,7 +622,7 @@ class DashboardModelView(SupersetModelView, DeleteMixin): # noqa 'dashboard_link': _('Dashboard'), 'dashboard_title': _('Title'), 'slug': _('Slug'), - 'slices': _('Charts'), + 'charts': _('Charts'), 'owners': _('Owners'), 'creator': _('Creator'), 'modified': _('Modified'), From 9d21f70a9b8e4d6a86ee52d9abd85901e12a512f Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Tue, 19 Mar 2019 11:58:11 -0700 Subject: [PATCH 26/86] handle null column_name in sqla and druid models (cherry picked from commit 2ff721ae072b8d69c5cabddc3e1a388a596b1b6f) --- superset/connectors/base/models.py | 6 ++++-- superset/connectors/druid/models.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index 39cc5853d6f66..958bea00e4dc9 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -86,7 +86,7 @@ def uid(self): @property def column_names(self): - return sorted([c.column_name for c in self.columns]) + return sorted([c.column_name for c in self.columns], key=lambda x: x or '') @property def columns_types(self): @@ -166,7 +166,9 @@ def select_star(self): def data(self): """Data representation of the datasource sent to the frontend""" order_by_choices = [] - for s in sorted(self.column_names): + # self.column_names return sorted column_names + for s in self.column_names: + s = str(s or '') order_by_choices.append((json.dumps([s, True]), s + ' [asc]')) order_by_choices.append((json.dumps([s, False]), s + ' [desc]')) diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 3b22ade1df366..144d4001876be 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -280,7 +280,7 @@ class DruidColumn(Model, BaseColumn): export_parent = 'datasource' def __repr__(self): - return self.column_name + return self.column_name or str(self.id) @property def expression(self): From bdbb3549fb3c968cc8a6b72ced290fb04a08d51d Mon Sep 17 00:00:00 2001 From: michellethomas Date: Mon, 25 Mar 2019 09:57:39 -0700 Subject: [PATCH 27/86] Use metric name instead of metric in filter box (#7106) (cherry picked from commit 003364e74ea70cad1a4a6e784933fe8bef4c78ec) --- superset/viz.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/viz.py b/superset/viz.py index 1a93adbfbe2b8..786fad11da942 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -1825,7 +1825,10 @@ def get_data(self, df): metric = flt.get('metric') df = self.dataframes.get(col) if metric: - df = df.sort_values(metric, ascending=flt.get('asc')) + df = df.sort_values( + utils.get_metric_name(metric), + ascending=flt.get('asc'), + ) d[col] = [{ 'id': row[0], 'text': row[0], From 87fb2df9f444cc405bce4c094c1e04562430a0a9 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 26 Mar 2019 13:27:16 -0700 Subject: [PATCH 28/86] Bump python lib croniter to an existing version (#7132) Package maintainers should really never delete packages, but it appears this happened with croniter and resulted in breaking our builds. This PR bumps to a more recent existing version of the library (cherry picked from commit 215ed392a11598eac228f57341dbfd232cf770e3) --- requirements.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 93181d6c9fbc7..2db602e629e01 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,7 @@ chardet==3.0.4 # via requests click==6.7 colorama==0.3.9 contextlib2==0.5.5 -croniter==0.3.26 +croniter==0.3.29 cryptography==2.4.2 decorator==4.3.0 # via retry defusedxml==0.5.0 # via python3-openid diff --git a/setup.py b/setup.py index 7d72dbe11d247..5ff7ec223a336 100644 --- a/setup.py +++ b/setup.py @@ -74,7 +74,7 @@ def get_git_sha(): 'click>=6.0, <7.0.0', # `click`>=7 forces "-" instead of "_" 'colorama', 'contextlib2', - 'croniter>=0.3.26', + 'croniter>=0.3.28', 'cryptography>=2.4.2', 'flask>=1.0.0, <2.0.0', 'flask-appbuilder>=1.12.3, <2.0.0', From b7fb15fe9287213fd0b801b252cb71b314263e4f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 28 Mar 2019 16:49:29 -0700 Subject: [PATCH 29/86] Revert PR #6933 (#7162) --- superset/views/core.py | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index f3b356db06928..8d9c8f86d9b6d 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -27,7 +27,7 @@ from flask import ( abort, flash, g, Markup, redirect, render_template, request, Response, url_for, ) -from flask_appbuilder import expose, Model, SimpleFormView +from flask_appbuilder import expose, SimpleFormView from flask_appbuilder.actions import action from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.decorators import has_access, has_access_api @@ -36,8 +36,7 @@ import pandas as pd import simplejson as json import sqlalchemy as sqla -from sqlalchemy import ( - and_, Column, create_engine, ForeignKey, Integer, MetaData, or_, Table, update) +from sqlalchemy import and_, create_engine, MetaData, or_, update from sqlalchemy.engine.url import make_url from sqlalchemy.exc import IntegrityError from werkzeug.routing import BaseConverter @@ -101,30 +100,13 @@ def is_owner(obj, user): return obj and user in obj.owners -SQLTable = Table( - 'tables', - Model.metadata, # pylint: disable=no-member - Column('id', Integer, primary_key=True), - Column('database_id', Integer, ForeignKey('dbs.id')), - extend_existing=True) - - class SliceFilter(SupersetFilter): def apply(self, query, func): # noqa if security_manager.all_datasource_access(): return query - + perms = self.get_view_menus('datasource_access') # TODO(bogdan): add `schema_access` support here - datasource_perms = self.get_view_menus('datasource_access') - query = ( - query.outerjoin(SQLTable, self.model.datasource_id == SQLTable.c.id) - .outerjoin(models.Database, models.Database.id == SQLTable.c.database_id) - .filter(or_( - models.Database.perm.in_(datasource_perms), - self.model.perm.in_(datasource_perms), - )) - ) - return query + return query.filter(self.model.perm.in_(perms)) class DashboardFilter(SupersetFilter): @@ -142,12 +124,7 @@ def apply(self, query, func): # noqa slice_ids_qry = ( db.session .query(Slice.id) - .outerjoin(SQLTable, Slice.datasource_id == SQLTable.c.id) - .outerjoin(models.Database, models.Database.id == SQLTable.c.database_id) - .filter(or_( - models.Database.perm.in_(datasource_perms), - Slice.perm.in_(datasource_perms), - )) + .filter(Slice.perm.in_(datasource_perms)) ) owner_ids_qry = ( db.session From 63f98dc158806946f2a30b192d3e086024c8d4e8 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 6 Mar 2019 14:07:40 +0200 Subject: [PATCH 30/86] Add decorator for etag cache --- superset/utils/decorators.py | 47 ++++++++++++++++++++++++++++++++++++ superset/views/core.py | 2 ++ 2 files changed, 49 insertions(+) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index b75b883eab0f9..07ee90a77d8d8 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -15,7 +15,12 @@ # specific language governing permissions and limitations # under the License. from contextlib2 import contextmanager +from datetime import datetime, timedelta +from functools import wraps +from flask import request + +from superset import cache from superset.utils.dates import now_as_float @@ -29,3 +34,45 @@ def stats_timing(stats_key, stats_logger): raise e finally: stats_logger.timing(stats_key, now_as_float() - start_ts) + + +def etag_cache(max_age): + """ + A decorator for caching views and handling etag conditional requests. + + The decorator caches the response, and returning headers for etag and last + modified. If the client makes a request that matches, the server will + return a "304 Not Mofified" status. + + """ + def decorator(f): + @wraps(f) + def wrapper(*args, **kwargs): + try: + # create key from args, kwargs and POST content + cache_key = wrapper.make_cache_key(f, request.form, *args, **kwargs) + response = cache.get(cache_key) + except Exception: + logger.exception('Exception possibly due to cache backend.') + return f(*args, **kwargs) + + if response is None: + response = f(*args, **kwargs) + response.cache_control.max_age = max_age + response.cache_control.public = True + response.last_modified = datetime.utcnow() + response.expires = response.last_modified + timedelta(seconds=max_age) + response.add_etag() + try: + cache.set(cache_key, response, timeout=max_age) + except Exception: + logger.exception("Exception possibly due to cache backend.") + + return response.make_conditional(request) + + wrapper.uncached = f + wrapper.cache_timeout = max_age + wrapper.make_cache_key = cache._memoize_make_cache_key(make_name=None, timeout=max_age) + return wrapper + + return decorator diff --git a/superset/views/core.py b/superset/views/core.py index 8d9c8f86d9b6d..d8e60eed1f674 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -58,6 +58,7 @@ from superset.utils import core as utils from superset.utils import dashboard_import_export from superset.utils.dates import now_as_float +from superset.utils.decorators import etag_cache from .base import ( api, BaseSupersetView, check_ownership, @@ -1198,6 +1199,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) + @etag_cache(60) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From b65dca6e6003f91426d6d995899e4d46c7d23fe1 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:03:32 +0200 Subject: [PATCH 31/86] Fetch charts with GET --- superset/assets/src/chart/Chart.jsx | 14 ++++++-------- superset/assets/src/chart/chartAction.js | 19 +++++++++++++++---- superset/utils/decorators.py | 5 ++--- superset/views/core.py | 2 +- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index bc19d63b6c624..f7103dc313d59 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -67,14 +67,12 @@ class Chart extends React.PureComponent { this.handleRenderContainerFailure = this.handleRenderContainerFailure.bind(this); } componentDidMount() { - if (this.props.triggerQuery) { - this.props.actions.runQuery( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); - } + this.props.actions.fetchChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); } handleRenderContainerFailure(error, info) { diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 93433c8235970..33d7795cc29f5 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -165,13 +165,12 @@ export function addChart(chart, key) { return { type: ADD_CHART, chart, key }; } -export const RUN_QUERY = 'RUN_QUERY'; -export function runQuery(formData, force = false, timeout = 60, key) { +export function exploreJSON(formData, force = false, timeout = 60, key, method) { return (dispatch) => { const { url, payload } = getExploreUrlAndPayload({ formData, endpointType: 'json', - force, + force: false, allowDomainSharding: true, }); const logStart = Logger.getTimestamp(); @@ -193,7 +192,9 @@ export function runQuery(formData, force = false, timeout = 60, key) { credentials: 'include', }; } - const queryPromise = SupersetClient.post(querySettings) + + const clientMethod = method === 'GET' ? SupersetClient.get : SupersetClient.post; + const queryPromise = clientMethod(querySettings) .then(({ json }) => { dispatch(logEvent(LOG_ACTIONS_LOAD_CHART, { slice_id: key, @@ -246,6 +247,16 @@ export function runQuery(formData, force = false, timeout = 60, key) { }; } +export const FETCH_CHART = 'FETCH_CHART'; +export function fetchChart(formData, force = false, timeout = 60, key) { + return exploreJSON(formData, force, timeout, key, 'GET'); +} + +export const RUN_QUERY = 'RUN_QUERY'; +export function runQuery(formData, force = false, timeout = 60, key) { + return exploreJSON(formData, force, timeout, key, 'POST'); +} + export function redirectSQLLab(formData) { return (dispatch) => { const { url } = getExploreUrlAndPayload({ formData, endpointType: 'query' }); diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 07ee90a77d8d8..43bab2f45a57d 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -49,14 +49,13 @@ def decorator(f): @wraps(f) def wrapper(*args, **kwargs): try: - # create key from args, kwargs and POST content - cache_key = wrapper.make_cache_key(f, request.form, *args, **kwargs) + cache_key = wrapper.make_cache_key(f, *args, **kwargs) response = cache.get(cache_key) except Exception: logger.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) - if response is None: + if response is None or request.method == 'POST': response = f(*args, **kwargs) response.cache_control.max_age = max_age response.cache_control.public = True diff --git a/superset/views/core.py b/superset/views/core.py index d8e60eed1f674..819bf7f7e1ccb 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1199,7 +1199,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(60) + #@etag_cache(60) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From a70a2f4f0d0a4302679c00a71223fc71f1eda69f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:16:56 +0200 Subject: [PATCH 32/86] Small fixes --- superset/assets/src/chart/chartAction.js | 2 +- superset/views/core.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 33d7795cc29f5..01b5e92b0c280 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -170,7 +170,7 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) const { url, payload } = getExploreUrlAndPayload({ formData, endpointType: 'json', - force: false, + force, allowDomainSharding: true, }); const logStart = Logger.getTimestamp(); diff --git a/superset/views/core.py b/superset/views/core.py index 819bf7f7e1ccb..de85b94d0f0e3 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -69,6 +69,7 @@ from .utils import bootstrap_user_data config = app.config +CACHE_DEFAULT_TIMEOUT = config.get('CACHE_DEFAULT_TIMEOUT', 0) stats_logger = config.get('STATS_LOGGER') log_this = models.Log.log_this DAR = models.DatasourceAccessRequest @@ -1199,7 +1200,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - #@etag_cache(60) + @etag_cache(CACHE_DEFAULT_TIMEOUT) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From afabd44fa95a9f5c605dc9baf648f42cbdb74229 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:39:11 +0200 Subject: [PATCH 33/86] Fix typo --- superset/utils/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 43bab2f45a57d..63fde0dace2b9 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -40,7 +40,7 @@ def etag_cache(max_age): """ A decorator for caching views and handling etag conditional requests. - The decorator caches the response, and returning headers for etag and last + The decorator caches the response, returning headers for etag and last modified. If the client makes a request that matches, the server will return a "304 Not Mofified" status. From c49dc70179c173279d057b95b21b0738a82f6b79 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 14:19:23 +0200 Subject: [PATCH 34/86] Compute correct cache key; fix logging --- superset/utils/decorators.py | 13 +++++++++---- superset/views/core.py | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 63fde0dace2b9..fc9aa5a2f1b30 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -17,6 +17,7 @@ from contextlib2 import contextmanager from datetime import datetime, timedelta from functools import wraps +import logging from flask import request @@ -36,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age): +def etag_cache(max_age, *additional_args): """ A decorator for caching views and handling etag conditional requests. @@ -49,10 +50,14 @@ def decorator(f): @wraps(f) def wrapper(*args, **kwargs): try: - cache_key = wrapper.make_cache_key(f, *args, **kwargs) + # build the cache key from the function arguments and any other + # additional GET arguments (like `form_data`, eg). + key_args = list(args[1:]) + key_args.extend(request.args.get(arg) for arg in additional_args) + cache_key = wrapper.make_cache_key(f, key_args, **kwargs) response = cache.get(cache_key) except Exception: - logger.exception('Exception possibly due to cache backend.') + logging.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) if response is None or request.method == 'POST': @@ -65,7 +70,7 @@ def wrapper(*args, **kwargs): try: cache.set(cache_key, response, timeout=max_age) except Exception: - logger.exception("Exception possibly due to cache backend.") + logging.exception("Exception possibly due to cache backend.") return response.make_conditional(request) diff --git a/superset/views/core.py b/superset/views/core.py index de85b94d0f0e3..2062d83568425 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1200,7 +1200,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT) + @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data') def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From ac07631443bd9584795685bc20e32dd5996f1bd2 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 16:52:41 +0200 Subject: [PATCH 35/86] Check perms on cached response --- superset/utils/decorators.py | 6 +++--- superset/views/core.py | 9 ++++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index fc9aa5a2f1b30..38cce02520806 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -37,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age, *additional_args): +def etag_cache(max_age, *additional_args, check_perms=bool): """ A decorator for caching views and handling etag conditional requests. @@ -49,10 +49,11 @@ def etag_cache(max_age, *additional_args): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): + check_perms(request) try: # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). - key_args = list(args[1:]) + key_args = list(args) key_args.extend(request.args.get(arg) for arg in additional_args) cache_key = wrapper.make_cache_key(f, key_args, **kwargs) response = cache.get(cache_key) @@ -62,7 +63,6 @@ def wrapper(*args, **kwargs): if response is None or request.method == 'POST': response = f(*args, **kwargs) - response.cache_control.max_age = max_age response.cache_control.public = True response.last_modified = datetime.utcnow() response.expires = response.last_modified + timedelta(seconds=max_age) diff --git a/superset/views/core.py b/superset/views/core.py index 2062d83568425..5a83a36f31e9f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -102,6 +102,13 @@ def is_owner(obj, user): return obj and user in obj.owners +def check_perms(request): + """Check if user can access a cached response from explore_json""" + slice_id = json.loads(request.args.get('form_data'))['slice_id'] + slc = db.session.query(models.Slice).filter_by(id=slice_id).one() + security_manager.assert_datasource_permission(slc.get_viz().datasource) + + class SliceFilter(SupersetFilter): def apply(self, query, func): # noqa if security_manager.all_datasource_access(): @@ -1200,7 +1207,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data') + @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data', check_perms=check_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From ea26a754259be42db685cfcefd16d458888c851c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 06:12:11 +0200 Subject: [PATCH 36/86] Revert change --- superset/assets/src/chart/Chart.jsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index f7103dc313d59..068c303b23783 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -67,12 +67,14 @@ class Chart extends React.PureComponent { this.handleRenderContainerFailure = this.handleRenderContainerFailure.bind(this); } componentDidMount() { - this.props.actions.fetchChart( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); + if (this.props.triggerQuery) { + this.props.actions.fetchChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } } handleRenderContainerFailure(error, info) { From 9d4acf7171e3f6f3920eecaea6a15cea2b4005d8 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 06:57:11 +0200 Subject: [PATCH 37/86] If perms fail, return naked response --- superset/utils/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 38cce02520806..fcc662a25908d 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -49,8 +49,8 @@ def etag_cache(max_age, *additional_args, check_perms=bool): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): - check_perms(request) try: + check_perms(request) # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) From 2a4613229958b75c3bd54a7cd0e746f9f7f2a411 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 07:51:09 +0200 Subject: [PATCH 38/86] Fix lint --- superset/utils/decorators.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index fcc662a25908d..2e0e884d6e4c5 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -14,11 +14,11 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from contextlib2 import contextmanager from datetime import datetime, timedelta from functools import wraps import logging +from contextlib2 import contextmanager from flask import request from superset import cache @@ -70,13 +70,14 @@ def wrapper(*args, **kwargs): try: cache.set(cache_key, response, timeout=max_age) except Exception: - logging.exception("Exception possibly due to cache backend.") + logging.exception('Exception possibly due to cache backend.') return response.make_conditional(request) wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key(make_name=None, timeout=max_age) + wrapper.make_cache_key = cache._memoize_make_cache_key( + make_name=None, timeout=max_age) return wrapper return decorator From c65a0dd7a1c2aa7b409298bbc99e67994d88d226 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 10:34:48 +0200 Subject: [PATCH 39/86] Compute cache key from all form data --- superset/utils/decorators.py | 7 ++++--- superset/views/core.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 2e0e884d6e4c5..c334a3a9d5a56 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -37,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age, *additional_args, check_perms=bool): +def etag_cache(max_age, check_perms=bool): """ A decorator for caching views and handling etag conditional requests. @@ -54,8 +54,9 @@ def wrapper(*args, **kwargs): # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) - key_args.extend(request.args.get(arg) for arg in additional_args) - cache_key = wrapper.make_cache_key(f, key_args, **kwargs) + key_kwargs = kwargs.copy() + key_kwargs.update(request.args) + cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) except Exception: logging.exception('Exception possibly due to cache backend.') diff --git a/superset/views/core.py b/superset/views/core.py index 5a83a36f31e9f..5d27da1f8c394 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1207,7 +1207,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data', check_perms=check_perms) + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From 35a7868d9b73b8abd14866b83833d21da9dcba00 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 11:42:31 +0200 Subject: [PATCH 40/86] Pass extra_filters in GET request --- superset/assets/src/explore/exploreUtils.js | 5 ++++- superset/views/core.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 48bb60a4fe362..3038b3a02837f 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -119,7 +119,10 @@ export function getExploreUrlAndPayload({ // Building the querystring (search) part of the URI const search = uri.search(true); if (formData.slice_id) { - search.form_data = safeStringify({ slice_id: formData.slice_id }); + search.form_data = safeStringify({ + slice_id: formData.slice_id, + extra_filters: formData.extra_filters, + }); } if (force) { search.force = 'true'; diff --git a/superset/views/core.py b/superset/views/core.py index 5d27da1f8c394..d77f30916b57a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1049,12 +1049,12 @@ def get_form_data(self, slice_id=None, use_slice_data=False): slice_id = form_data.get('slice_id') or slice_id slc = None - # Check if form data only contains slice_id - contains_only_slc_id = not any(key != 'slice_id' for key in form_data) + # Check if form data only contains slice_id and extra_fiters + valid_slice_id = all(key in ['slice_id', 'extra_filters'] for key in form_data) # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id - if slice_id and (use_slice_data or contains_only_slc_id): + # or if form_data only contains slice_id and extra_filters + if slice_id and (use_slice_data or valid_slice_id): slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() if slc: slice_form_data = slc.form_data.copy() From 4dd19e09337f7aabf6a24df84bc6bd70ee2c9ab1 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 14:02:51 +0200 Subject: [PATCH 41/86] Fix pylint --- superset/utils/decorators.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index c334a3a9d5a56..089ca05418621 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -58,7 +58,7 @@ def wrapper(*args, **kwargs): key_kwargs.update(request.args) cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) - except Exception: + except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) @@ -70,14 +70,14 @@ def wrapper(*args, **kwargs): response.add_etag() try: cache.set(cache_key, response, timeout=max_age) - except Exception: + except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') return response.make_conditional(request) wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key( + wrapper.make_cache_key = cache._memoize_make_cache_key( # pylint: disable=protected-access make_name=None, timeout=max_age) return wrapper From debd5e0df51ac34d75a43a201ed29b5e21233026 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 14:34:27 +0200 Subject: [PATCH 42/86] Fix flake8 --- superset/utils/decorators.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 089ca05418621..0579cdd9ae4e2 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -77,8 +77,9 @@ def wrapper(*args, **kwargs): wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key( # pylint: disable=protected-access - make_name=None, timeout=max_age) + wrapper.make_cache_key = \ + cache._memoize_make_cache_key( # pylint: disable=protected-access + make_name=None, timeout=max_age) return wrapper return decorator From e49cd034966a180ec2fa616d57f4ea37ea0c770c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Sun, 17 Mar 2019 06:00:40 +0200 Subject: [PATCH 43/86] Use ETags even if no cache is set --- superset/utils/decorators.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 0579cdd9ae4e2..8b183e472875a 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -45,6 +45,9 @@ def etag_cache(max_age, check_perms=bool): modified. If the client makes a request that matches, the server will return a "304 Not Mofified" status. + If no cache is set, the decorator will still set the ETag header, and + handle conditional requests. + """ def decorator(f): @wraps(f) @@ -60,7 +63,7 @@ def wrapper(*args, **kwargs): response = cache.get(cache_key) except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') - return f(*args, **kwargs) + response = None if response is None or request.method == 'POST': response = f(*args, **kwargs) @@ -75,11 +78,13 @@ def wrapper(*args, **kwargs): return response.make_conditional(request) - wrapper.uncached = f - wrapper.cache_timeout = max_age - wrapper.make_cache_key = \ - cache._memoize_make_cache_key( # pylint: disable=protected-access - make_name=None, timeout=max_age) + if cache: + wrapper.uncached = f + wrapper.cache_timeout = max_age + wrapper.make_cache_key = \ + cache._memoize_make_cache_key( # pylint: disable=protected-access + make_name=None, timeout=max_age) + return wrapper return decorator From ca65c0c9676695d7982528963a686c7e01a873ad Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Sun, 17 Mar 2019 06:01:01 +0200 Subject: [PATCH 44/86] Handle adhoc filters --- superset/assets/src/explore/exploreUtils.js | 1 + superset/views/core.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 3038b3a02837f..7a04e8a1494a8 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -122,6 +122,7 @@ export function getExploreUrlAndPayload({ search.form_data = safeStringify({ slice_id: formData.slice_id, extra_filters: formData.extra_filters, + adhoc_filters: formData.adhoc_filters, }); } if (force) { diff --git a/superset/views/core.py b/superset/views/core.py index d77f30916b57a..c5b46cecdc1b3 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1049,11 +1049,12 @@ def get_form_data(self, slice_id=None, use_slice_data=False): slice_id = form_data.get('slice_id') or slice_id slc = None - # Check if form data only contains slice_id and extra_fiters - valid_slice_id = all(key in ['slice_id', 'extra_filters'] for key in form_data) + # Check if form data only contains slice_id and additional filters + valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] + valid_slice_id = all(key in valid_keys for key in form_data) # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id and extra_filters + # or if form_data only contains slice_id and additional filters if slice_id and (use_slice_data or valid_slice_id): slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() if slc: From 58c84db7f0cd637fb18ad2e8f80578b0d5c821ce Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 21 Mar 2019 17:42:30 -0700 Subject: [PATCH 45/86] Raise in debug mode --- superset/utils/decorators.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 8b183e472875a..2fbc3f3048b3e 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -21,7 +21,7 @@ from contextlib2 import contextmanager from flask import request -from superset import cache +from superset import app, cache from superset.utils.dates import now_as_float @@ -62,6 +62,8 @@ def wrapper(*args, **kwargs): cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) except Exception: # pylint: disable=broad-except + if app.debug: + raise logging.exception('Exception possibly due to cache backend.') response = None From 60e849ba34fe7182dde76774963d4f3c2082c9a6 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 21 Mar 2019 17:42:57 -0700 Subject: [PATCH 46/86] Rename actions --- superset/assets/src/chart/Chart.jsx | 2 +- superset/assets/src/chart/chartAction.js | 26 +++++++++++++++---- .../src/dashboard/components/Dashboard.jsx | 4 +-- .../src/dashboard/containers/Dashboard.jsx | 4 +-- .../explore/components/ExploreChartHeader.jsx | 6 ++--- .../components/ExploreViewContainer.jsx | 5 ++-- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 068c303b23783..88de2551f9f7f 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -68,7 +68,7 @@ class Chart extends React.PureComponent { } componentDidMount() { if (this.props.triggerQuery) { - this.props.actions.fetchChart( + this.props.actions.getSavedChart( this.props.formData, false, this.props.timeout, diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 01b5e92b0c280..2f1ce11dc750c 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -247,13 +247,29 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) }; } -export const FETCH_CHART = 'FETCH_CHART'; -export function fetchChart(formData, force = false, timeout = 60, key) { +export const GET_SAVED_CHART = 'GET_SAVED_CHART'; +export function getSavedChart(formData, force = false, timeout = 60, key) { + /* + * Perform a GET request to `/explore_json`. + * + * This will return the payload of a saved chart, optionally filtered by + * ad-hoc or extra filters from dashboards. Eg: + * + * GET /explore_json?{"chart_id":1} + * GET /explore_json?{"chart_id":1,"extra_filters":"..."} + * + */ return exploreJSON(formData, force, timeout, key, 'GET'); } -export const RUN_QUERY = 'RUN_QUERY'; -export function runQuery(formData, force = false, timeout = 60, key) { +export const POST_CHART_FORM_DATA = 'POST_CHART_FORM_DATA'; +export function postChartFormData(formData, force = false, timeout = 60, key) { + /* + * Perform a POST request to `/explore_json`. + * + * This will post the form data to the endpoint, returning a new chart. + * + */ return exploreJSON(formData, force, timeout, key, 'POST'); } @@ -283,6 +299,6 @@ export function refreshChart(chart, force, timeout) { if (!chart.latestQueryFormData || Object.keys(chart.latestQueryFormData).length === 0) { return; } - dispatch(runQuery(chart.latestQueryFormData, force, timeout, chart.id)); + dispatch(postChartFormData(chart.latestQueryFormData, force, timeout, chart.id)); }; } diff --git a/superset/assets/src/dashboard/components/Dashboard.jsx b/superset/assets/src/dashboard/components/Dashboard.jsx index dc1f05479ccb9..32cc3768c5d7f 100644 --- a/superset/assets/src/dashboard/components/Dashboard.jsx +++ b/superset/assets/src/dashboard/components/Dashboard.jsx @@ -40,7 +40,7 @@ const propTypes = { actions: PropTypes.shape({ addSliceToDashboard: PropTypes.func.isRequired, removeSliceFromDashboard: PropTypes.func.isRequired, - runQuery: PropTypes.func.isRequired, + postChartFormData: PropTypes.func.isRequired, logEvent: PropTypes.func.isRequired, }).isRequired, dashboardInfo: dashboardInfoPropShape.isRequired, @@ -156,7 +156,7 @@ class Dashboard extends React.PureComponent { sliceId: chart.id, }); - this.props.actions.runQuery( + this.props.actions.postChartFormData( updatedFormData, false, this.props.timeout, diff --git a/superset/assets/src/dashboard/containers/Dashboard.jsx b/superset/assets/src/dashboard/containers/Dashboard.jsx index 865ec40ac0293..e5cf4fb59ea0e 100644 --- a/superset/assets/src/dashboard/containers/Dashboard.jsx +++ b/superset/assets/src/dashboard/containers/Dashboard.jsx @@ -25,7 +25,7 @@ import { addSliceToDashboard, removeSliceFromDashboard, } from '../actions/dashboardState'; -import { runQuery } from '../../chart/chartAction'; +import { postChartFormData } from '../../chart/chartAction'; import { logEvent } from '../../logger/actions'; import getLoadStatsPerTopLevelComponent from '../util/logging/getLoadStatsPerTopLevelComponent'; @@ -64,7 +64,7 @@ function mapDispatchToProps(dispatch) { { addSliceToDashboard, removeSliceFromDashboard, - runQuery, + postChartFormData, logEvent, }, dispatch, diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index d2f3b983024b0..31c74ad1a70c3 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -50,8 +50,8 @@ const propTypes = { }; class ExploreChartHeader extends React.PureComponent { - runQuery() { - this.props.actions.runQuery(this.props.form_data, true, + postChartFormData() { + this.props.actions.postChartFormData(this.props.form_data, true, this.props.timeout, this.props.chart.id); } @@ -142,7 +142,7 @@ class ExploreChartHeader extends React.PureComponent { />} {chartSucceeded && queryResponse && queryResponse.is_cached && } Date: Fri, 22 Mar 2019 11:11:38 -0700 Subject: [PATCH 47/86] Fix integration tests --- .../cypress/integration/dashboard/controls.js | 2 +- .../integration/dashboard/edit_mode.js | 2 +- .../cypress/integration/dashboard/filter.js | 2 +- .../cypress/integration/dashboard/load.js | 2 +- .../cypress/integration/dashboard/save.js | 2 +- .../integration/explore/control.test.js | 36 +++++++++++-------- .../cypress/integration/explore/link.test.js | 9 ++--- 7 files changed, 31 insertions(+), 24 deletions(-) diff --git a/superset/assets/cypress/integration/dashboard/controls.js b/superset/assets/cypress/integration/dashboard/controls.js index fb103bde44c58..3e218d2ca7a07 100644 --- a/superset/assets/cypress/integration/dashboard/controls.js +++ b/superset/assets/cypress/integration/dashboard/controls.js @@ -39,7 +39,7 @@ export default () => describe('top-level controls', () => { .forEach((id) => { const sliceRequest = `getJson_${id}`; sliceRequests.push(`@${sliceRequest}`); - cy.route('POST', `/superset/explore_json/?form_data={"slice_id":${id}}`).as(sliceRequest); + cy.route('GET', `/superset/explore_json/?form_data={"slice_id":${id}}`).as(sliceRequest); const forceRefresh = `getJson_${id}_force`; forceRefreshRequests.push(`@${forceRefresh}`); diff --git a/superset/assets/cypress/integration/dashboard/edit_mode.js b/superset/assets/cypress/integration/dashboard/edit_mode.js index e58e7df53bc1e..d9395d276d968 100644 --- a/superset/assets/cypress/integration/dashboard/edit_mode.js +++ b/superset/assets/cypress/integration/dashboard/edit_mode.js @@ -29,7 +29,7 @@ export default () => describe('edit mode', () => { const dashboard = bootstrapData.dashboard_data; const boxplotChartId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'box_plot')).slice_id; const boxplotRequest = `/superset/explore_json/?form_data={"slice_id":${boxplotChartId}}`; - cy.route('POST', boxplotRequest).as('boxplotRequest'); + cy.route('GET', boxplotRequest).as('boxplotRequest'); }); cy.get('.dashboard-header').contains('Edit dashboard').click(); diff --git a/superset/assets/cypress/integration/dashboard/filter.js b/superset/assets/cypress/integration/dashboard/filter.js index 157cbe8685dc7..6ec1c926154d7 100644 --- a/superset/assets/cypress/integration/dashboard/filter.js +++ b/superset/assets/cypress/integration/dashboard/filter.js @@ -40,7 +40,7 @@ export default () => describe('dashboard filter', () => { const aliases = []; const filterRoute = `/superset/explore_json/?form_data={"slice_id":${filterId}}`; - cy.route('POST', filterRoute).as('fetchFilter'); + cy.route('GET', filterRoute).as('fetchFilter'); cy.wait('@fetchFilter'); sliceIds .filter(id => (parseInt(id, 10) !== filterId)) diff --git a/superset/assets/cypress/integration/dashboard/load.js b/superset/assets/cypress/integration/dashboard/load.js index 30c9d325dac62..0dbe1ff147abc 100644 --- a/superset/assets/cypress/integration/dashboard/load.js +++ b/superset/assets/cypress/integration/dashboard/load.js @@ -34,7 +34,7 @@ export default () => describe('load', () => { // then define routes and create alias for each requests slices.forEach((slice) => { const alias = `getJson_${slice.slice_id}`; - cy.route('POST', `/superset/explore_json/?form_data={"slice_id":${slice.slice_id}}`).as(alias); + cy.route('GET', `/superset/explore_json/?form_data={"slice_id":${slice.slice_id}}`).as(alias); aliases.push(`@${alias}`); }); }); diff --git a/superset/assets/cypress/integration/dashboard/save.js b/superset/assets/cypress/integration/dashboard/save.js index 1c673b0c715a4..d144a71ca5780 100644 --- a/superset/assets/cypress/integration/dashboard/save.js +++ b/superset/assets/cypress/integration/dashboard/save.js @@ -57,7 +57,7 @@ export default () => describe('save', () => { // should have box_plot chart const boxplotRequest = `/superset/explore_json/?form_data={"slice_id":${boxplotChartId}}`; - cy.route('POST', boxplotRequest).as('boxplotRequest'); + cy.route('GET', boxplotRequest).as('boxplotRequest'); cy.wait('@boxplotRequest'); cy.get('.grid-container .box_plot').should('be.exist'); diff --git a/superset/assets/cypress/integration/explore/control.test.js b/superset/assets/cypress/integration/explore/control.test.js index ba4636c47c76c..711ab783425cd 100644 --- a/superset/assets/cypress/integration/explore/control.test.js +++ b/superset/assets/cypress/integration/explore/control.test.js @@ -26,7 +26,8 @@ describe('Groupby', () => { cy.server(); cy.login(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@getJson' }); @@ -36,7 +37,7 @@ describe('Groupby', () => { cy.get('.VirtualizedSelectFocusedOption').click(); }); cy.get('button.query').click(); - cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' }); + cy.verifySliceSuccess({ waitAlias: '@postJson', chartSelector: 'svg' }); }); }); @@ -44,7 +45,8 @@ describe('AdhocMetrics', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Clear metric and set simple adhoc metric', () => { @@ -74,7 +76,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', querySubstring: metricName, chartSelector: 'svg', }); @@ -105,7 +107,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', querySubstring: metric, chartSelector: 'svg', }); @@ -137,7 +139,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -147,7 +149,8 @@ describe('AdhocFilters', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Set simple adhoc filter', () => { @@ -177,7 +180,7 @@ describe('AdhocFilters', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -206,7 +209,7 @@ describe('AdhocFilters', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -217,7 +220,8 @@ describe('Advanced analytics', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Create custom time compare', () => { @@ -240,7 +244,7 @@ describe('Advanced analytics', () => { }); cy.get('button.query').click(); - cy.wait('@getJson'); + cy.wait('@postJson'); cy.reload(); cy.verifySliceSuccess({ waitAlias: '@getJson', @@ -257,7 +261,8 @@ describe('Annotations', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Create formula annotation y-axis goal line', () => { @@ -280,7 +285,7 @@ describe('Annotations', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); @@ -292,7 +297,8 @@ describe('Time range filter', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Defaults to the correct tab for time_range params', () => { @@ -304,7 +310,7 @@ describe('Time range filter', () => { }; cy.visitChartByParams(JSON.stringify(formData)); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=time_range]').within(() => { cy.get('span.label').click(); diff --git a/superset/assets/cypress/integration/explore/link.test.js b/superset/assets/cypress/integration/explore/link.test.js index 024612f53b7d8..36b56ce5d5ac7 100644 --- a/superset/assets/cypress/integration/explore/link.test.js +++ b/superset/assets/cypress/integration/explore/link.test.js @@ -26,7 +26,8 @@ describe('Test explore links', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Open and close view query modal', () => { @@ -35,7 +36,7 @@ describe('Test explore links', () => { cy.get('button#query').click(); cy.get('span').contains('View query').parent().click(); - cy.wait('@getJson').then(() => { + cy.wait('@postJson').then(() => { cy.get('code'); }); cy.get('.modal-header').within(() => { @@ -83,7 +84,7 @@ describe('Test explore links', () => { const newChartName = 'Test chart'; cy.visitChartByParams(JSON.stringify(formData)); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.url().then((url) => { cy.get('button[data-target="#save_modal"]').click(); cy.get('.modal-content').within(() => { @@ -109,7 +110,7 @@ describe('Test explore links', () => { cy.get('.modal-content').within(() => { cy.get('button#btn_modal_save').click(); }); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.request(`/chart/api/read?_flt_3_slice_name=${chartName}`).then((response) => { cy.request('DELETE', `/chart/api/delete/${response.body.pks[0]}`); }); From 8ed131d56053b3dae38be13f00dc59b5ea872384 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:12:17 -0700 Subject: [PATCH 48/86] Do POST request on new charts --- superset/assets/src/chart/Chart.jsx | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 88de2551f9f7f..8cb615361e2c5 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -68,12 +68,23 @@ class Chart extends React.PureComponent { } componentDidMount() { if (this.props.triggerQuery) { - this.props.actions.getSavedChart( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); + if (this.props.chartId > 0) { + // Load saved chart with a GET request + this.props.actions.getSavedChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } else { + // Create chart with POST request + this.props.actions.postChartFormData( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } } } From e96784a3b17851f0a3f33435f056cb399f84fc88 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:13:51 -0700 Subject: [PATCH 49/86] Set extra/adhoc filters only in GET requests --- superset/assets/src/chart/chartAction.js | 1 + superset/assets/src/explore/exploreUtils.js | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 2f1ce11dc750c..3909dadd85967 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -172,6 +172,7 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) endpointType: 'json', force, allowDomainSharding: true, + method, }); const logStart = Logger.getTimestamp(); const controller = new AbortController(); diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 7a04e8a1494a8..4785933385bb2 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -95,6 +95,7 @@ export function getExploreUrlAndPayload({ curUrl = null, requestParams = {}, allowDomainSharding = false, + method = 'POST', }) { if (!formData.datasource) { return null; @@ -118,12 +119,18 @@ export function getExploreUrlAndPayload({ // Building the querystring (search) part of the URI const search = uri.search(true); - if (formData.slice_id) { - search.form_data = safeStringify({ - slice_id: formData.slice_id, - extra_filters: formData.extra_filters, - adhoc_filters: formData.adhoc_filters, - }); + const { slice_id, extra_filters, adhoc_filters } = formData; + if (slice_id) { + const form_data = { slice_id }; + if (method === 'GET') { + if (extra_filters && extra_filters.length) { + form_data.extra_filters = extra_filters; + } + if (adhoc_filters && adhoc_filters.length) { + form_data.adhoc_filters = adhoc_filters; + } + } + search.form_data = safeStringify(form_data); } if (force) { search.force = 'true'; From 3f49312cda9477c26c2b5c3eaf61de1dcc1a5761 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:14:22 -0700 Subject: [PATCH 50/86] Raise if check_perms fails --- superset/utils/decorators.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 2fbc3f3048b3e..7db95f2354331 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -52,8 +52,10 @@ def etag_cache(max_age, check_perms=bool): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): + # check if the user can access the resource + check_perms(*args, **kwargs) + try: - check_perms(request) # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) From 532dc2e748de882ca2acd25cd5fe52eb6e6ab57b Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:14:48 -0700 Subject: [PATCH 51/86] Refactor auth --- superset/views/core.py | 202 +++++++++++++--------------------------- superset/views/utils.py | 118 ++++++++++++++++++++++- 2 files changed, 182 insertions(+), 138 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index c5b46cecdc1b3..1c1e58ddcb405 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -66,7 +66,7 @@ get_error_msg, handle_api_exception, json_error_response, json_success, SupersetFilter, SupersetModelView, YamlExportMixin, ) -from .utils import bootstrap_user_data +from .utils import bootstrap_user_data, get_datasource_info, get_form_data, get_viz config = app.config CACHE_DEFAULT_TIMEOUT = config.get('CACHE_DEFAULT_TIMEOUT', 0) @@ -102,11 +102,44 @@ def is_owner(obj, user): return obj and user in obj.owners -def check_perms(request): - """Check if user can access a cached response from explore_json""" - slice_id = json.loads(request.args.get('form_data'))['slice_id'] - slc = db.session.query(models.Slice).filter_by(id=slice_id).one() - security_manager.assert_datasource_permission(slc.get_viz().datasource) +def check_datasource_perms(self, datasource_type=None, datasource_id=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. + + """ + form_data = get_form_data()[0] + datasource_id, datasource_type = get_datasource_info( + datasource_id, datasource_type, form_data) + 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) + + +def check_slice_perms(self, slice_id): + """ + Check if user can access a cached response from slice_json. + + This function takes `self` since it must have the same signature as the + the decorated method. + + """ + form_data, slc = get_form_data(slice_id, use_slice_data=True) + datasource_type = slc.datasource.type + datasource_id = slc.datasource.id + 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) class SliceFilter(SupersetFilter): @@ -1012,90 +1045,10 @@ def clean_fulfilled_requests(session): session.commit() return redirect('/accessrequestsmodelview/list/') - def get_form_data(self, slice_id=None, use_slice_data=False): - form_data = {} - post_data = request.form.get('form_data') - request_args_data = request.args.get('form_data') - # Supporting POST - if post_data: - form_data.update(json.loads(post_data)) - # request params can overwrite post body - if request_args_data: - form_data.update(json.loads(request_args_data)) - - url_id = request.args.get('r') - if url_id: - saved_url = db.session.query(models.Url).filter_by(id=url_id).first() - if saved_url: - url_str = parse.unquote_plus( - saved_url.url.split('?')[1][10:], encoding='utf-8', errors=None) - url_form_data = json.loads(url_str) - # allow form_date in request override saved url - url_form_data.update(form_data) - form_data = url_form_data - - if request.args.get('viz_type'): - # Converting old URLs - form_data = cast_form_data(form_data) - - form_data = { - k: v - for k, v in form_data.items() - if k not in FORM_DATA_KEY_BLACKLIST - } - - # When a slice_id is present, load from DB and override - # the form_data from the DB with the other form_data provided - slice_id = form_data.get('slice_id') or slice_id - slc = None - - # Check if form data only contains slice_id and additional filters - valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] - valid_slice_id = all(key in valid_keys for key in form_data) - - # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id and additional filters - if slice_id and (use_slice_data or valid_slice_id): - slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() - if slc: - slice_form_data = slc.form_data.copy() - slice_form_data.update(form_data) - form_data = slice_form_data - - update_time_range(form_data) - - return form_data, slc - - def get_viz( - self, - slice_id=None, - form_data=None, - datasource_type=None, - datasource_id=None, - force=False, - ): - if slice_id: - slc = ( - db.session.query(models.Slice) - .filter_by(id=slice_id) - .one() - ) - return slc.get_viz() - else: - viz_type = form_data.get('viz_type', 'table') - datasource = ConnectorRegistry.get_datasource( - datasource_type, datasource_id, db.session) - viz_obj = viz.viz_types[viz_type]( - datasource, - form_data=form_data, - force=force, - ) - return viz_obj - @has_access @expose('/slice//') def slice(self, slice_id): - form_data, slc = self.get_form_data(slice_id, use_slice_data=True) + form_data, slc = get_form_data(slice_id, use_slice_data=True) if not slc: abort(404) endpoint = '/superset/explore/?form_data={}'.format( @@ -1139,18 +1092,7 @@ def get_samples(self, viz_obj): }) def generate_json( - self, datasource_type, datasource_id, form_data, - csv=False, query=False, force=False, results=False, - samples=False, - ): - viz_obj = self.get_viz( - datasource_type=datasource_type, - datasource_id=datasource_id, - form_data=form_data, - force=force, - ) - security_manager.assert_datasource_permission(viz_obj.datasource) - + self, viz_obj, csv=False, query=False, results=False, samples=False): if csv: return CsvResponse( viz_obj.get_csv(), @@ -1174,21 +1116,25 @@ def generate_json( @api @has_access_api @expose('/slice_json/') + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_slice_perms) def slice_json(self, slice_id): - form_data, slc = self.get_form_data(slice_id, use_slice_data=True) + form_data, slc = get_form_data(slice_id, use_slice_data=True) datasource_type = slc.datasource.type datasource_id = slc.datasource.id - - return self.generate_json(datasource_type=datasource_type, - datasource_id=datasource_id, - form_data=form_data) + viz_obj = get_viz( + datasource_type=datasource_type, + datasource_id=datasource_id, + form_data=form_data, + force=False, + ) + return self.generate_json(viz_obj) @log_this @api @has_access_api @expose('/annotation_json/') def annotation_json(self, layer_id): - form_data = self.get_form_data()[0] + form_data = get_form_data()[0] form_data['layer_id'] = layer_id form_data['filters'] = [{'col': 'layer_id', 'op': '==', @@ -1208,7 +1154,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_perms) + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_datasource_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data @@ -1225,18 +1171,21 @@ def explore_json(self, datasource_type=None, datasource_id=None): samples = request.args.get('samples') == 'true' force = request.args.get('force') == 'true' - form_data = self.get_form_data()[0] - datasource_id, datasource_type = self.datasource_info( + form_data = get_form_data()[0] + datasource_id, datasource_type = get_datasource_info( datasource_id, datasource_type, form_data) - - return self.generate_json( + viz_obj = get_viz( datasource_type=datasource_type, datasource_id=datasource_id, form_data=form_data, + force=force, + ) + + return self.generate_json( + viz_obj, csv=csv, query=query, results=results, - force=force, samples=samples, ) @@ -1262,34 +1211,15 @@ def explorev2(self, datasource_type, datasource_id): datasource_id=datasource_id, **request.args)) - @staticmethod - def datasource_info(datasource_id, datasource_type, form_data): - """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""" - 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 not datasource_id: - raise Exception( - 'The datasource associated with this chart no longer exists') - datasource_id = int(datasource_id) - return datasource_id, datasource_type - @log_this @has_access @expose('/explore///', methods=['GET', 'POST']) @expose('/explore/', methods=['GET', 'POST']) def explore(self, datasource_type=None, datasource_id=None): user_id = g.user.get_id() if g.user else None - form_data, slc = self.get_form_data(use_slice_data=True) + form_data, slc = get_form_data(use_slice_data=True) - datasource_id, datasource_type = self.datasource_info( + datasource_id, datasource_type = get_datasource_info( datasource_id, datasource_type, form_data) error_redirect = '/chart/list/' @@ -1415,7 +1345,7 @@ def save_or_overwrite_slice( """Save or overwrite a slice""" slice_name = args.get('slice_name') action = args.get('action') - form_data, _ = self.get_form_data() + form_data, _ = get_form_data() if action in ('saveas'): if 'slice_id' in form_data: @@ -2086,8 +2016,8 @@ def warm_up_cache(self): for slc in slices: try: - form_data = self.get_form_data(slc.id, use_slice_data=True)[0] - obj = self.get_viz( + form_data = get_form_data(slc.id, use_slice_data=True)[0] + obj = get_viz( datasource_type=slc.datasource.type, datasource_id=slc.datasource.id, form_data=form_data, @@ -2853,7 +2783,7 @@ def slice_query(self, slice_id): This method exposes an API endpoint to get the database query string for this slice """ - viz_obj = self.get_viz(slice_id) + viz_obj = get_viz(slice_id) security_manager.assert_datasource_permission(viz_obj.datasource) return self.get_query_string_response(viz_obj) diff --git a/superset/views/utils.py b/superset/views/utils.py index 9b2bef04ced14..0ef4831bb2b6c 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -16,11 +16,25 @@ # under the License. # pylint: disable=C,R,W from collections import defaultdict +from urllib import parse -from flask import g +from flask import g, request from flask_appbuilder.security.sqla import models as ab_models +import simplejson as json -from superset import db +from superset import app, db, viz +from superset.connectors.connector_registry import ConnectorRegistry +from superset.legacy import cast_form_data, update_time_range +import superset.models.core as models + + +FORM_DATA_KEY_BLACKLIST = [] +if not app.config.get('ENABLE_JAVASCRIPT_CONTROLS'): + FORM_DATA_KEY_BLACKLIST = [ + 'js_tooltip', + 'js_onclick_href', + 'js_data_mutator', + ] def bootstrap_user_data(username=None, include_perms=False): @@ -76,3 +90,103 @@ def get_permissions(user): ] return roles, permissions + + +def get_viz( + slice_id=None, + form_data=None, + datasource_type=None, + datasource_id=None, + force=False, +): + if slice_id: + slc = ( + db.session.query(models.Slice) + .filter_by(id=slice_id) + .one() + ) + return slc.get_viz() + else: + viz_type = form_data.get('viz_type', 'table') + datasource = ConnectorRegistry.get_datasource( + datasource_type, datasource_id, db.session) + viz_obj = viz.viz_types[viz_type]( + datasource, + form_data=form_data, + force=force, + ) + return viz_obj + + +def get_form_data(slice_id=None, use_slice_data=False): + form_data = {} + post_data = request.form.get('form_data') + request_args_data = request.args.get('form_data') + # Supporting POST + if post_data: + form_data.update(json.loads(post_data)) + # request params can overwrite post body + if request_args_data: + form_data.update(json.loads(request_args_data)) + + url_id = request.args.get('r') + if url_id: + saved_url = db.session.query(models.Url).filter_by(id=url_id).first() + if saved_url: + url_str = parse.unquote_plus( + saved_url.url.split('?')[1][10:], encoding='utf-8', errors=None) + url_form_data = json.loads(url_str) + # allow form_date in request override saved url + url_form_data.update(form_data) + form_data = url_form_data + + if request.args.get('viz_type'): + # Converting old URLs + form_data = cast_form_data(form_data) + + form_data = { + k: v + for k, v in form_data.items() + if k not in FORM_DATA_KEY_BLACKLIST + } + + # When a slice_id is present, load from DB and override + # the form_data from the DB with the other form_data provided + slice_id = form_data.get('slice_id') or slice_id + slc = None + + # Check if form data only contains slice_id and additional filters + valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] + valid_slice_id = all(key in valid_keys for key in form_data) + + # Include the slice_form_data if request from explore or slice calls + # or if form_data only contains slice_id and additional filters + if slice_id and (use_slice_data or valid_slice_id): + slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() + if slc: + slice_form_data = slc.form_data.copy() + slice_form_data.update(form_data) + form_data = slice_form_data + + update_time_range(form_data) + + return form_data, slc + + +def get_datasource_info(datasource_id, datasource_type, form_data): + """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""" + 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 not datasource_id: + raise Exception( + 'The datasource associated with this chart no longer exists') + datasource_id = int(datasource_id) + return datasource_id, datasource_type From 26313253bf8fd1e7047e5c13fc07e3aaf46698cd Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:38:30 -0700 Subject: [PATCH 52/86] Fix flake8 --- superset/views/core.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 1c1e58ddcb405..4b7d14e4e3542 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -50,7 +50,6 @@ from superset.exceptions import SupersetException from superset.forms import CsvToDatabaseForm from superset.jinja_context import get_template_processor -from superset.legacy import cast_form_data, update_time_range import superset.models.core as models from superset.models.sql_lab import Query from superset.models.user_attributes import UserAttribute @@ -105,10 +104,10 @@ def is_owner(obj, user): def check_datasource_perms(self, datasource_type=None, datasource_id=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. - + """ form_data = get_form_data()[0] datasource_id, datasource_type = get_datasource_info( @@ -125,10 +124,10 @@ def check_datasource_perms(self, datasource_type=None, datasource_id=None): def check_slice_perms(self, slice_id): """ Check if user can access a cached response from slice_json. - + This function takes `self` since it must have the same signature as the the decorated method. - + """ form_data, slc = get_form_data(slice_id, use_slice_data=True) datasource_type = slc.datasource.type @@ -1092,7 +1091,7 @@ def get_samples(self, viz_obj): }) def generate_json( - self, viz_obj, csv=False, query=False, results=False, samples=False): + self, viz_obj, csv=False, query=False, results=False, samples=False): if csv: return CsvResponse( viz_obj.get_csv(), From 7fb193174724bbfe900bd95be450c6b90acc6165 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 12:07:08 -0700 Subject: [PATCH 53/86] Fix js unit tests --- .../spec/javascripts/chart/chartActions_spec.js | 14 +++++++------- .../dashboard/components/Dashboard_spec.jsx | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/superset/assets/spec/javascripts/chart/chartActions_spec.js b/superset/assets/spec/javascripts/chart/chartActions_spec.js index 09f618c6c7d6b..a4a832632e3de 100644 --- a/superset/assets/spec/javascripts/chart/chartActions_spec.js +++ b/superset/assets/spec/javascripts/chart/chartActions_spec.js @@ -51,7 +51,7 @@ describe('chart actions', () => { }); it('should dispatch CHART_UPDATE_STARTED action before the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success @@ -64,7 +64,7 @@ describe('chart actions', () => { }); it('should dispatch TRIGGER_QUERY action with the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -76,7 +76,7 @@ describe('chart actions', () => { }); it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -88,7 +88,7 @@ describe('chart actions', () => { }); it('should dispatch logEvent async action', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -104,7 +104,7 @@ describe('chart actions', () => { }); it('should dispatch CHART_UPDATE_SUCCEEDED action upon success', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -120,7 +120,7 @@ describe('chart actions', () => { fetchMock.post(MOCK_URL, () => unresolvingPromise, { overwriteRoutes: true }); const timeoutInSec = 1 / 1000; - const actionThunk = actions.runQuery({}, false, timeoutInSec); + const actionThunk = actions.postChartFormData({}, false, timeoutInSec); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, fail @@ -136,7 +136,7 @@ describe('chart actions', () => { fetchMock.post(MOCK_URL, { throws: { statusText: 'misc error' } }, { overwriteRoutes: true }); const timeoutInSec = 1 / 1000; - const actionThunk = actions.runQuery({}, false, timeoutInSec); + const actionThunk = actions.postChartFormData({}, false, timeoutInSec); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, fail diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx index 523b835d475bf..de637cd2ada9a 100644 --- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx @@ -39,7 +39,7 @@ describe('Dashboard', () => { actions: { addSliceToDashboard() {}, removeSliceFromDashboard() {}, - runQuery() {}, + postChartFormData() {}, logEvent() {}, }, initMessages: [], @@ -82,15 +82,15 @@ describe('Dashboard', () => { }, }; - it('should call runQuery for all non-exempt slices', () => { + it('should call postChartFormData for all non-exempt slices', () => { const wrapper = setup({ charts: overrideCharts, slices: overrideSlices }); - const spy = sinon.spy(props.actions, 'runQuery'); + const spy = sinon.spy(props.actions, 'postChartFormData'); wrapper.instance().refreshExcept('1001'); spy.restore(); expect(spy.callCount).toBe(Object.keys(overrideCharts).length - 1); }); - it('should not call runQuery for filter_immune_slices', () => { + it('should not call postChartFormData for filter_immune_slices', () => { const wrapper = setup({ charts: overrideCharts, dashboardInfo: { @@ -103,7 +103,7 @@ describe('Dashboard', () => { }, }, }); - const spy = sinon.spy(props.actions, 'runQuery'); + const spy = sinon.spy(props.actions, 'postChartFormData'); wrapper.instance().refreshExcept(); spy.restore(); expect(spy.callCount).toBe(0); From cc15133e50fe0db9cfe04910daf18ca94803aad4 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 12:23:22 -0700 Subject: [PATCH 54/86] Fix js unit tests that fail in lyftga --- .../assets/spec/javascripts/components/AsyncSelect_spec.jsx | 2 +- .../assets/spec/javascripts/welcome/DashboardTable_spec.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx index 6918e1b94ccc7..dd466ceb6b47e 100644 --- a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx +++ b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx @@ -76,7 +76,7 @@ describe('AsyncSelect', () => { ); setTimeout(() => { - expect(fetchMock.calls(dataGlob)).toHaveLength(1); + expect(fetchMock.calls(dataGlob)).toHaveLength(3); expect(onChangeSpy.mock.calls).toHaveLength(0); done(); }); diff --git a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx index e989f9a207aea..3cbb1075f29d6 100644 --- a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx +++ b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx @@ -54,7 +54,7 @@ describe('DashboardTable', () => { const wrapper = setup(); setTimeout(() => { - expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(1); + expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(2); // there's a delay between response and updating state, so manually set it // rather than adding a timeout which could introduce flakiness wrapper.setState({ dashaboards: mockDashboards }); From cb1e7cbaa0877cf19505d1cd02c34a254fa5ccf4 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 13:22:06 -0700 Subject: [PATCH 55/86] Fix js --- .../assets/spec/javascripts/components/AsyncSelect_spec.jsx | 2 +- .../assets/spec/javascripts/welcome/DashboardTable_spec.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx index dd466ceb6b47e..6918e1b94ccc7 100644 --- a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx +++ b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx @@ -76,7 +76,7 @@ describe('AsyncSelect', () => { ); setTimeout(() => { - expect(fetchMock.calls(dataGlob)).toHaveLength(3); + expect(fetchMock.calls(dataGlob)).toHaveLength(1); expect(onChangeSpy.mock.calls).toHaveLength(0); done(); }); diff --git a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx index 3cbb1075f29d6..e989f9a207aea 100644 --- a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx +++ b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx @@ -54,7 +54,7 @@ describe('DashboardTable', () => { const wrapper = setup(); setTimeout(() => { - expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(2); + expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(1); // there's a delay between response and updating state, so manually set it // rather than adding a timeout which could introduce flakiness wrapper.setState({ dashaboards: mockDashboards }); From 2f97bf91ea396855f1f4187dbcf269e2a1148e6e Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Tue, 5 Mar 2019 01:11:59 -0800 Subject: [PATCH 56/86] Sparkline dates aren't formatting in Time Series Table (#6976) * Exclude venv for python linter to ignore * Fix NaN error --- superset/assets/src/visualizations/TimeTable/TimeTable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx index c895d46187717..dac3a0768ed73 100644 --- a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx +++ b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx @@ -149,7 +149,7 @@ class TimeTable extends React.PureComponent { renderTooltip={({ index }) => (
{formatNumber(column.d3format, sparkData[index])} -
{formatTime(column.dateFormat, moment.utc(entries[index].time).toDate())}
+
{formatTime(column.dateFormat, new Date(entries[index].time))}
)} /> From 32088cce0c3219f76c6d591a3db792a40fc4e037 Mon Sep 17 00:00:00 2001 From: michellethomas Date: Wed, 13 Mar 2019 09:34:51 -0700 Subject: [PATCH 57/86] Changing time table viz to pass formatTime a date (#7020) (cherry picked from commit 7f3c145b1f5a4e2d8b95982119503e98772e2c47) --- superset/assets/src/visualizations/TimeTable/TimeTable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx index dac3a0768ed73..c895d46187717 100644 --- a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx +++ b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx @@ -149,7 +149,7 @@ class TimeTable extends React.PureComponent { renderTooltip={({ index }) => (
{formatNumber(column.d3format, sparkData[index])} -
{formatTime(column.dateFormat, new Date(entries[index].time))}
+
{formatTime(column.dateFormat, moment.utc(entries[index].time).toDate())}
)} /> From 23dadc740a5251a5cf2149ebacdd034c9b60a46c Mon Sep 17 00:00:00 2001 From: Christine Chambers Date: Mon, 25 Mar 2019 15:19:43 -0700 Subject: [PATCH 58/86] SQL editor layout makeover (#7102) This PR includes the following layout and css tweaks: - Using flex to layout the north and south sub panes of query pane so resizing works properly in both Chrome and Firefox - Removal of necessary wrapper divs and tweaking of css in sql lab so we can scroll to the bottom of both the table list and the results pane - Make sql lab's content not overflow vertically and layout the query result area to eliminate double scroll bars - css tweaks on the basic.html page so the loading animation appears in the center of the page across the board (cherry picked from commit 71f1bbd2ec59b99d6ba6d9a4a2f9cfceaf922b80) --- superset/assets/package-lock.json | 41 +++++++++---------------------- 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json index 18c3acf1cac86..6901767288724 100644 --- a/superset/assets/package-lock.json +++ b/superset/assets/package-lock.json @@ -8684,8 +8684,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -8706,14 +8705,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -8728,20 +8725,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -8858,8 +8852,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -8871,7 +8864,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -8886,7 +8878,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -8894,14 +8885,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -8920,7 +8909,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -9001,8 +8989,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -9014,7 +9001,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -9100,8 +9086,7 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -9137,7 +9122,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -9157,7 +9141,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -9201,14 +9184,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, From dc5950758d4be548b9a0f6d55e96786de86db80c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 6 Mar 2019 14:07:40 +0200 Subject: [PATCH 59/86] Add decorator for etag cache --- superset/utils/decorators.py | 47 ++++++++++++++++++++++++++++++++++++ superset/views/core.py | 2 ++ 2 files changed, 49 insertions(+) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index b75b883eab0f9..07ee90a77d8d8 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -15,7 +15,12 @@ # specific language governing permissions and limitations # under the License. from contextlib2 import contextmanager +from datetime import datetime, timedelta +from functools import wraps +from flask import request + +from superset import cache from superset.utils.dates import now_as_float @@ -29,3 +34,45 @@ def stats_timing(stats_key, stats_logger): raise e finally: stats_logger.timing(stats_key, now_as_float() - start_ts) + + +def etag_cache(max_age): + """ + A decorator for caching views and handling etag conditional requests. + + The decorator caches the response, and returning headers for etag and last + modified. If the client makes a request that matches, the server will + return a "304 Not Mofified" status. + + """ + def decorator(f): + @wraps(f) + def wrapper(*args, **kwargs): + try: + # create key from args, kwargs and POST content + cache_key = wrapper.make_cache_key(f, request.form, *args, **kwargs) + response = cache.get(cache_key) + except Exception: + logger.exception('Exception possibly due to cache backend.') + return f(*args, **kwargs) + + if response is None: + response = f(*args, **kwargs) + response.cache_control.max_age = max_age + response.cache_control.public = True + response.last_modified = datetime.utcnow() + response.expires = response.last_modified + timedelta(seconds=max_age) + response.add_etag() + try: + cache.set(cache_key, response, timeout=max_age) + except Exception: + logger.exception("Exception possibly due to cache backend.") + + return response.make_conditional(request) + + wrapper.uncached = f + wrapper.cache_timeout = max_age + wrapper.make_cache_key = cache._memoize_make_cache_key(make_name=None, timeout=max_age) + return wrapper + + return decorator diff --git a/superset/views/core.py b/superset/views/core.py index ad75c3b88f328..7b00a09ef1e5e 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -58,6 +58,7 @@ from superset.utils import core as utils from superset.utils import dashboard_import_export from superset.utils.dates import now_as_float +from superset.utils.decorators import etag_cache from .base import ( api, BaseSupersetView, check_ownership, @@ -1207,6 +1208,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) + @etag_cache(60) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From a745f325ed654e90f3a1f8f08ff88ef53ac19731 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:03:32 +0200 Subject: [PATCH 60/86] Fetch charts with GET --- superset/assets/src/chart/Chart.jsx | 14 ++++++-------- superset/assets/src/chart/chartAction.js | 19 +++++++++++++++---- superset/utils/decorators.py | 5 ++--- superset/views/core.py | 2 +- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index bc19d63b6c624..f7103dc313d59 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -67,14 +67,12 @@ class Chart extends React.PureComponent { this.handleRenderContainerFailure = this.handleRenderContainerFailure.bind(this); } componentDidMount() { - if (this.props.triggerQuery) { - this.props.actions.runQuery( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); - } + this.props.actions.fetchChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); } handleRenderContainerFailure(error, info) { diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 93433c8235970..33d7795cc29f5 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -165,13 +165,12 @@ export function addChart(chart, key) { return { type: ADD_CHART, chart, key }; } -export const RUN_QUERY = 'RUN_QUERY'; -export function runQuery(formData, force = false, timeout = 60, key) { +export function exploreJSON(formData, force = false, timeout = 60, key, method) { return (dispatch) => { const { url, payload } = getExploreUrlAndPayload({ formData, endpointType: 'json', - force, + force: false, allowDomainSharding: true, }); const logStart = Logger.getTimestamp(); @@ -193,7 +192,9 @@ export function runQuery(formData, force = false, timeout = 60, key) { credentials: 'include', }; } - const queryPromise = SupersetClient.post(querySettings) + + const clientMethod = method === 'GET' ? SupersetClient.get : SupersetClient.post; + const queryPromise = clientMethod(querySettings) .then(({ json }) => { dispatch(logEvent(LOG_ACTIONS_LOAD_CHART, { slice_id: key, @@ -246,6 +247,16 @@ export function runQuery(formData, force = false, timeout = 60, key) { }; } +export const FETCH_CHART = 'FETCH_CHART'; +export function fetchChart(formData, force = false, timeout = 60, key) { + return exploreJSON(formData, force, timeout, key, 'GET'); +} + +export const RUN_QUERY = 'RUN_QUERY'; +export function runQuery(formData, force = false, timeout = 60, key) { + return exploreJSON(formData, force, timeout, key, 'POST'); +} + export function redirectSQLLab(formData) { return (dispatch) => { const { url } = getExploreUrlAndPayload({ formData, endpointType: 'query' }); diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 07ee90a77d8d8..43bab2f45a57d 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -49,14 +49,13 @@ def decorator(f): @wraps(f) def wrapper(*args, **kwargs): try: - # create key from args, kwargs and POST content - cache_key = wrapper.make_cache_key(f, request.form, *args, **kwargs) + cache_key = wrapper.make_cache_key(f, *args, **kwargs) response = cache.get(cache_key) except Exception: logger.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) - if response is None: + if response is None or request.method == 'POST': response = f(*args, **kwargs) response.cache_control.max_age = max_age response.cache_control.public = True diff --git a/superset/views/core.py b/superset/views/core.py index 7b00a09ef1e5e..9f0f70a0a6e23 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1208,7 +1208,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(60) + #@etag_cache(60) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From 1931441bf853edf20ce5fda2e17d1752ab41e0ce Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:16:56 +0200 Subject: [PATCH 61/86] Small fixes --- superset/assets/src/chart/chartAction.js | 2 +- superset/views/core.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 33d7795cc29f5..01b5e92b0c280 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -170,7 +170,7 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) const { url, payload } = getExploreUrlAndPayload({ formData, endpointType: 'json', - force: false, + force, allowDomainSharding: true, }); const logStart = Logger.getTimestamp(); diff --git a/superset/views/core.py b/superset/views/core.py index 9f0f70a0a6e23..5b588606d6330 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -69,6 +69,7 @@ from .utils import bootstrap_user_data config = app.config +CACHE_DEFAULT_TIMEOUT = config.get('CACHE_DEFAULT_TIMEOUT', 0) stats_logger = config.get('STATS_LOGGER') log_this = models.Log.log_this DAR = models.DatasourceAccessRequest @@ -1208,7 +1209,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - #@etag_cache(60) + @etag_cache(CACHE_DEFAULT_TIMEOUT) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From ff037e5f6064c0590ebb771454086b1cd7421b6e Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 13:39:11 +0200 Subject: [PATCH 62/86] Fix typo --- superset/utils/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 43bab2f45a57d..63fde0dace2b9 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -40,7 +40,7 @@ def etag_cache(max_age): """ A decorator for caching views and handling etag conditional requests. - The decorator caches the response, and returning headers for etag and last + The decorator caches the response, returning headers for etag and last modified. If the client makes a request that matches, the server will return a "304 Not Mofified" status. From 6a4adb3b618cee737132bb5cc081d0a8cc25dde0 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 14:19:23 +0200 Subject: [PATCH 63/86] Compute correct cache key; fix logging --- superset/utils/decorators.py | 13 +++++++++---- superset/views/core.py | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 63fde0dace2b9..fc9aa5a2f1b30 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -17,6 +17,7 @@ from contextlib2 import contextmanager from datetime import datetime, timedelta from functools import wraps +import logging from flask import request @@ -36,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age): +def etag_cache(max_age, *additional_args): """ A decorator for caching views and handling etag conditional requests. @@ -49,10 +50,14 @@ def decorator(f): @wraps(f) def wrapper(*args, **kwargs): try: - cache_key = wrapper.make_cache_key(f, *args, **kwargs) + # build the cache key from the function arguments and any other + # additional GET arguments (like `form_data`, eg). + key_args = list(args[1:]) + key_args.extend(request.args.get(arg) for arg in additional_args) + cache_key = wrapper.make_cache_key(f, key_args, **kwargs) response = cache.get(cache_key) except Exception: - logger.exception('Exception possibly due to cache backend.') + logging.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) if response is None or request.method == 'POST': @@ -65,7 +70,7 @@ def wrapper(*args, **kwargs): try: cache.set(cache_key, response, timeout=max_age) except Exception: - logger.exception("Exception possibly due to cache backend.") + logging.exception("Exception possibly due to cache backend.") return response.make_conditional(request) diff --git a/superset/views/core.py b/superset/views/core.py index 5b588606d6330..d8693d09623fe 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1209,7 +1209,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT) + @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data') def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From 94360341ddb01190a51a2ced224d7b1fa9436351 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 13 Mar 2019 16:52:41 +0200 Subject: [PATCH 64/86] Check perms on cached response --- superset/utils/decorators.py | 6 +++--- superset/views/core.py | 9 ++++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index fc9aa5a2f1b30..38cce02520806 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -37,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age, *additional_args): +def etag_cache(max_age, *additional_args, check_perms=bool): """ A decorator for caching views and handling etag conditional requests. @@ -49,10 +49,11 @@ def etag_cache(max_age, *additional_args): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): + check_perms(request) try: # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). - key_args = list(args[1:]) + key_args = list(args) key_args.extend(request.args.get(arg) for arg in additional_args) cache_key = wrapper.make_cache_key(f, key_args, **kwargs) response = cache.get(cache_key) @@ -62,7 +63,6 @@ def wrapper(*args, **kwargs): if response is None or request.method == 'POST': response = f(*args, **kwargs) - response.cache_control.max_age = max_age response.cache_control.public = True response.last_modified = datetime.utcnow() response.expires = response.last_modified + timedelta(seconds=max_age) diff --git a/superset/views/core.py b/superset/views/core.py index d8693d09623fe..c6a1e2faa1210 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -102,6 +102,13 @@ def is_owner(obj, user): return obj and user in obj.owners +def check_perms(request): + """Check if user can access a cached response from explore_json""" + slice_id = json.loads(request.args.get('form_data'))['slice_id'] + slc = db.session.query(models.Slice).filter_by(id=slice_id).one() + security_manager.assert_datasource_permission(slc.get_viz().datasource) + + class DatabaseFilter(SupersetFilter): def apply(self, query, func): # noqa if security_manager.all_database_access(): @@ -1209,7 +1216,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data') + @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data', check_perms=check_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From 9df4dc2796f1483b409c6b446de28fa82d4beb9b Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 06:12:11 +0200 Subject: [PATCH 65/86] Revert change --- superset/assets/src/chart/Chart.jsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index f7103dc313d59..068c303b23783 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -67,12 +67,14 @@ class Chart extends React.PureComponent { this.handleRenderContainerFailure = this.handleRenderContainerFailure.bind(this); } componentDidMount() { - this.props.actions.fetchChart( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); + if (this.props.triggerQuery) { + this.props.actions.fetchChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } } handleRenderContainerFailure(error, info) { From e0fbb64c4211bb57c24436c58fe3369a4490d84a Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 06:57:11 +0200 Subject: [PATCH 66/86] If perms fail, return naked response --- superset/utils/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 38cce02520806..fcc662a25908d 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -49,8 +49,8 @@ def etag_cache(max_age, *additional_args, check_perms=bool): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): - check_perms(request) try: + check_perms(request) # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) From c093b033839f10573599dc7c179b2aa9a478e0f4 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 14 Mar 2019 07:51:09 +0200 Subject: [PATCH 67/86] Fix lint --- superset/utils/decorators.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index fcc662a25908d..2e0e884d6e4c5 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -14,11 +14,11 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from contextlib2 import contextmanager from datetime import datetime, timedelta from functools import wraps import logging +from contextlib2 import contextmanager from flask import request from superset import cache @@ -70,13 +70,14 @@ def wrapper(*args, **kwargs): try: cache.set(cache_key, response, timeout=max_age) except Exception: - logging.exception("Exception possibly due to cache backend.") + logging.exception('Exception possibly due to cache backend.') return response.make_conditional(request) wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key(make_name=None, timeout=max_age) + wrapper.make_cache_key = cache._memoize_make_cache_key( + make_name=None, timeout=max_age) return wrapper return decorator From ef3afd20cfe434c611f9e9aae83626e932894612 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 10:34:48 +0200 Subject: [PATCH 68/86] Compute cache key from all form data --- superset/utils/decorators.py | 7 ++++--- superset/views/core.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 2e0e884d6e4c5..c334a3a9d5a56 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -37,7 +37,7 @@ def stats_timing(stats_key, stats_logger): stats_logger.timing(stats_key, now_as_float() - start_ts) -def etag_cache(max_age, *additional_args, check_perms=bool): +def etag_cache(max_age, check_perms=bool): """ A decorator for caching views and handling etag conditional requests. @@ -54,8 +54,9 @@ def wrapper(*args, **kwargs): # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) - key_args.extend(request.args.get(arg) for arg in additional_args) - cache_key = wrapper.make_cache_key(f, key_args, **kwargs) + key_kwargs = kwargs.copy() + key_kwargs.update(request.args) + cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) except Exception: logging.exception('Exception possibly due to cache backend.') diff --git a/superset/views/core.py b/superset/views/core.py index c6a1e2faa1210..a1ffedd44f30c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1216,7 +1216,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, 'form_data', check_perms=check_perms) + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data From 86e4b935690b7100b781fb2404a8df7f1965e8c3 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 11:42:31 +0200 Subject: [PATCH 69/86] Pass extra_filters in GET request --- superset/assets/src/explore/exploreUtils.js | 5 ++++- superset/views/core.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 48bb60a4fe362..3038b3a02837f 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -119,7 +119,10 @@ export function getExploreUrlAndPayload({ // Building the querystring (search) part of the URI const search = uri.search(true); if (formData.slice_id) { - search.form_data = safeStringify({ slice_id: formData.slice_id }); + search.form_data = safeStringify({ + slice_id: formData.slice_id, + extra_filters: formData.extra_filters, + }); } if (force) { search.force = 'true'; diff --git a/superset/views/core.py b/superset/views/core.py index a1ffedd44f30c..7b0a3d84603f9 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1058,12 +1058,12 @@ def get_form_data(self, slice_id=None, use_slice_data=False): slice_id = form_data.get('slice_id') or slice_id slc = None - # Check if form data only contains slice_id - contains_only_slc_id = not any(key != 'slice_id' for key in form_data) + # Check if form data only contains slice_id and extra_fiters + valid_slice_id = all(key in ['slice_id', 'extra_filters'] for key in form_data) # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id - if slice_id and (use_slice_data or contains_only_slc_id): + # or if form_data only contains slice_id and extra_filters + if slice_id and (use_slice_data or valid_slice_id): slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() if slc: slice_form_data = slc.form_data.copy() From 68412bf7501b09108b4138185760423d1f098efd Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 14:02:51 +0200 Subject: [PATCH 70/86] Fix pylint --- superset/utils/decorators.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index c334a3a9d5a56..089ca05418621 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -58,7 +58,7 @@ def wrapper(*args, **kwargs): key_kwargs.update(request.args) cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) - except Exception: + except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') return f(*args, **kwargs) @@ -70,14 +70,14 @@ def wrapper(*args, **kwargs): response.add_etag() try: cache.set(cache_key, response, timeout=max_age) - except Exception: + except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') return response.make_conditional(request) wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key( + wrapper.make_cache_key = cache._memoize_make_cache_key( # pylint: disable=protected-access make_name=None, timeout=max_age) return wrapper From 69eeb1d7255d30972747bee4e0ab178857b407b6 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 15 Mar 2019 14:34:27 +0200 Subject: [PATCH 71/86] Fix flake8 --- superset/utils/decorators.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 089ca05418621..0579cdd9ae4e2 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -77,8 +77,9 @@ def wrapper(*args, **kwargs): wrapper.uncached = f wrapper.cache_timeout = max_age - wrapper.make_cache_key = cache._memoize_make_cache_key( # pylint: disable=protected-access - make_name=None, timeout=max_age) + wrapper.make_cache_key = \ + cache._memoize_make_cache_key( # pylint: disable=protected-access + make_name=None, timeout=max_age) return wrapper return decorator From fe8cc653d806b0fa3c6f3bcc7e6c8f51a267af1f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Sun, 17 Mar 2019 06:00:40 +0200 Subject: [PATCH 72/86] Use ETags even if no cache is set --- superset/utils/decorators.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 0579cdd9ae4e2..8b183e472875a 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -45,6 +45,9 @@ def etag_cache(max_age, check_perms=bool): modified. If the client makes a request that matches, the server will return a "304 Not Mofified" status. + If no cache is set, the decorator will still set the ETag header, and + handle conditional requests. + """ def decorator(f): @wraps(f) @@ -60,7 +63,7 @@ def wrapper(*args, **kwargs): response = cache.get(cache_key) except Exception: # pylint: disable=broad-except logging.exception('Exception possibly due to cache backend.') - return f(*args, **kwargs) + response = None if response is None or request.method == 'POST': response = f(*args, **kwargs) @@ -75,11 +78,13 @@ def wrapper(*args, **kwargs): return response.make_conditional(request) - wrapper.uncached = f - wrapper.cache_timeout = max_age - wrapper.make_cache_key = \ - cache._memoize_make_cache_key( # pylint: disable=protected-access - make_name=None, timeout=max_age) + if cache: + wrapper.uncached = f + wrapper.cache_timeout = max_age + wrapper.make_cache_key = \ + cache._memoize_make_cache_key( # pylint: disable=protected-access + make_name=None, timeout=max_age) + return wrapper return decorator From f8200a183db5ffedcf4b511fd26bac081f353716 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Sun, 17 Mar 2019 06:01:01 +0200 Subject: [PATCH 73/86] Handle adhoc filters --- superset/assets/src/explore/exploreUtils.js | 1 + superset/views/core.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 3038b3a02837f..7a04e8a1494a8 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -122,6 +122,7 @@ export function getExploreUrlAndPayload({ search.form_data = safeStringify({ slice_id: formData.slice_id, extra_filters: formData.extra_filters, + adhoc_filters: formData.adhoc_filters, }); } if (force) { diff --git a/superset/views/core.py b/superset/views/core.py index 7b0a3d84603f9..e7b8f8a58394b 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1058,11 +1058,12 @@ def get_form_data(self, slice_id=None, use_slice_data=False): slice_id = form_data.get('slice_id') or slice_id slc = None - # Check if form data only contains slice_id and extra_fiters - valid_slice_id = all(key in ['slice_id', 'extra_filters'] for key in form_data) + # Check if form data only contains slice_id and additional filters + valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] + valid_slice_id = all(key in valid_keys for key in form_data) # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id and extra_filters + # or if form_data only contains slice_id and additional filters if slice_id and (use_slice_data or valid_slice_id): slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() if slc: From b3c488dc5817fee58823ee7b8eec9e114a73845d Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 21 Mar 2019 17:42:30 -0700 Subject: [PATCH 74/86] Raise in debug mode --- superset/utils/decorators.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 8b183e472875a..2fbc3f3048b3e 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -21,7 +21,7 @@ from contextlib2 import contextmanager from flask import request -from superset import cache +from superset import app, cache from superset.utils.dates import now_as_float @@ -62,6 +62,8 @@ def wrapper(*args, **kwargs): cache_key = wrapper.make_cache_key(f, *key_args, **key_kwargs) response = cache.get(cache_key) except Exception: # pylint: disable=broad-except + if app.debug: + raise logging.exception('Exception possibly due to cache backend.') response = None From 89f0192435e8acb0550c616051b019d7c0d39787 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 21 Mar 2019 17:42:57 -0700 Subject: [PATCH 75/86] Rename actions --- superset/assets/src/chart/Chart.jsx | 2 +- superset/assets/src/chart/chartAction.js | 26 +++++++++++++++---- .../src/dashboard/components/Dashboard.jsx | 4 +-- .../src/dashboard/containers/Dashboard.jsx | 4 +-- .../explore/components/ExploreChartHeader.jsx | 6 ++--- .../components/ExploreViewContainer.jsx | 5 ++-- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 068c303b23783..88de2551f9f7f 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -68,7 +68,7 @@ class Chart extends React.PureComponent { } componentDidMount() { if (this.props.triggerQuery) { - this.props.actions.fetchChart( + this.props.actions.getSavedChart( this.props.formData, false, this.props.timeout, diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 01b5e92b0c280..2f1ce11dc750c 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -247,13 +247,29 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) }; } -export const FETCH_CHART = 'FETCH_CHART'; -export function fetchChart(formData, force = false, timeout = 60, key) { +export const GET_SAVED_CHART = 'GET_SAVED_CHART'; +export function getSavedChart(formData, force = false, timeout = 60, key) { + /* + * Perform a GET request to `/explore_json`. + * + * This will return the payload of a saved chart, optionally filtered by + * ad-hoc or extra filters from dashboards. Eg: + * + * GET /explore_json?{"chart_id":1} + * GET /explore_json?{"chart_id":1,"extra_filters":"..."} + * + */ return exploreJSON(formData, force, timeout, key, 'GET'); } -export const RUN_QUERY = 'RUN_QUERY'; -export function runQuery(formData, force = false, timeout = 60, key) { +export const POST_CHART_FORM_DATA = 'POST_CHART_FORM_DATA'; +export function postChartFormData(formData, force = false, timeout = 60, key) { + /* + * Perform a POST request to `/explore_json`. + * + * This will post the form data to the endpoint, returning a new chart. + * + */ return exploreJSON(formData, force, timeout, key, 'POST'); } @@ -283,6 +299,6 @@ export function refreshChart(chart, force, timeout) { if (!chart.latestQueryFormData || Object.keys(chart.latestQueryFormData).length === 0) { return; } - dispatch(runQuery(chart.latestQueryFormData, force, timeout, chart.id)); + dispatch(postChartFormData(chart.latestQueryFormData, force, timeout, chart.id)); }; } diff --git a/superset/assets/src/dashboard/components/Dashboard.jsx b/superset/assets/src/dashboard/components/Dashboard.jsx index dc1f05479ccb9..32cc3768c5d7f 100644 --- a/superset/assets/src/dashboard/components/Dashboard.jsx +++ b/superset/assets/src/dashboard/components/Dashboard.jsx @@ -40,7 +40,7 @@ const propTypes = { actions: PropTypes.shape({ addSliceToDashboard: PropTypes.func.isRequired, removeSliceFromDashboard: PropTypes.func.isRequired, - runQuery: PropTypes.func.isRequired, + postChartFormData: PropTypes.func.isRequired, logEvent: PropTypes.func.isRequired, }).isRequired, dashboardInfo: dashboardInfoPropShape.isRequired, @@ -156,7 +156,7 @@ class Dashboard extends React.PureComponent { sliceId: chart.id, }); - this.props.actions.runQuery( + this.props.actions.postChartFormData( updatedFormData, false, this.props.timeout, diff --git a/superset/assets/src/dashboard/containers/Dashboard.jsx b/superset/assets/src/dashboard/containers/Dashboard.jsx index 865ec40ac0293..e5cf4fb59ea0e 100644 --- a/superset/assets/src/dashboard/containers/Dashboard.jsx +++ b/superset/assets/src/dashboard/containers/Dashboard.jsx @@ -25,7 +25,7 @@ import { addSliceToDashboard, removeSliceFromDashboard, } from '../actions/dashboardState'; -import { runQuery } from '../../chart/chartAction'; +import { postChartFormData } from '../../chart/chartAction'; import { logEvent } from '../../logger/actions'; import getLoadStatsPerTopLevelComponent from '../util/logging/getLoadStatsPerTopLevelComponent'; @@ -64,7 +64,7 @@ function mapDispatchToProps(dispatch) { { addSliceToDashboard, removeSliceFromDashboard, - runQuery, + postChartFormData, logEvent, }, dispatch, diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index d2f3b983024b0..31c74ad1a70c3 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -50,8 +50,8 @@ const propTypes = { }; class ExploreChartHeader extends React.PureComponent { - runQuery() { - this.props.actions.runQuery(this.props.form_data, true, + postChartFormData() { + this.props.actions.postChartFormData(this.props.form_data, true, this.props.timeout, this.props.chart.id); } @@ -142,7 +142,7 @@ class ExploreChartHeader extends React.PureComponent { />} {chartSucceeded && queryResponse && queryResponse.is_cached && } Date: Fri, 22 Mar 2019 11:11:38 -0700 Subject: [PATCH 76/86] Fix integration tests --- .../cypress/integration/dashboard/controls.js | 2 +- .../integration/dashboard/edit_mode.js | 2 +- .../cypress/integration/dashboard/filter.js | 2 +- .../cypress/integration/dashboard/load.js | 2 +- .../cypress/integration/dashboard/save.js | 2 +- .../integration/explore/control.test.js | 36 +++++++++++-------- .../cypress/integration/explore/link.test.js | 9 ++--- 7 files changed, 31 insertions(+), 24 deletions(-) diff --git a/superset/assets/cypress/integration/dashboard/controls.js b/superset/assets/cypress/integration/dashboard/controls.js index fb103bde44c58..3e218d2ca7a07 100644 --- a/superset/assets/cypress/integration/dashboard/controls.js +++ b/superset/assets/cypress/integration/dashboard/controls.js @@ -39,7 +39,7 @@ export default () => describe('top-level controls', () => { .forEach((id) => { const sliceRequest = `getJson_${id}`; sliceRequests.push(`@${sliceRequest}`); - cy.route('POST', `/superset/explore_json/?form_data={"slice_id":${id}}`).as(sliceRequest); + cy.route('GET', `/superset/explore_json/?form_data={"slice_id":${id}}`).as(sliceRequest); const forceRefresh = `getJson_${id}_force`; forceRefreshRequests.push(`@${forceRefresh}`); diff --git a/superset/assets/cypress/integration/dashboard/edit_mode.js b/superset/assets/cypress/integration/dashboard/edit_mode.js index e58e7df53bc1e..d9395d276d968 100644 --- a/superset/assets/cypress/integration/dashboard/edit_mode.js +++ b/superset/assets/cypress/integration/dashboard/edit_mode.js @@ -29,7 +29,7 @@ export default () => describe('edit mode', () => { const dashboard = bootstrapData.dashboard_data; const boxplotChartId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'box_plot')).slice_id; const boxplotRequest = `/superset/explore_json/?form_data={"slice_id":${boxplotChartId}}`; - cy.route('POST', boxplotRequest).as('boxplotRequest'); + cy.route('GET', boxplotRequest).as('boxplotRequest'); }); cy.get('.dashboard-header').contains('Edit dashboard').click(); diff --git a/superset/assets/cypress/integration/dashboard/filter.js b/superset/assets/cypress/integration/dashboard/filter.js index 157cbe8685dc7..6ec1c926154d7 100644 --- a/superset/assets/cypress/integration/dashboard/filter.js +++ b/superset/assets/cypress/integration/dashboard/filter.js @@ -40,7 +40,7 @@ export default () => describe('dashboard filter', () => { const aliases = []; const filterRoute = `/superset/explore_json/?form_data={"slice_id":${filterId}}`; - cy.route('POST', filterRoute).as('fetchFilter'); + cy.route('GET', filterRoute).as('fetchFilter'); cy.wait('@fetchFilter'); sliceIds .filter(id => (parseInt(id, 10) !== filterId)) diff --git a/superset/assets/cypress/integration/dashboard/load.js b/superset/assets/cypress/integration/dashboard/load.js index 30c9d325dac62..0dbe1ff147abc 100644 --- a/superset/assets/cypress/integration/dashboard/load.js +++ b/superset/assets/cypress/integration/dashboard/load.js @@ -34,7 +34,7 @@ export default () => describe('load', () => { // then define routes and create alias for each requests slices.forEach((slice) => { const alias = `getJson_${slice.slice_id}`; - cy.route('POST', `/superset/explore_json/?form_data={"slice_id":${slice.slice_id}}`).as(alias); + cy.route('GET', `/superset/explore_json/?form_data={"slice_id":${slice.slice_id}}`).as(alias); aliases.push(`@${alias}`); }); }); diff --git a/superset/assets/cypress/integration/dashboard/save.js b/superset/assets/cypress/integration/dashboard/save.js index 1c673b0c715a4..d144a71ca5780 100644 --- a/superset/assets/cypress/integration/dashboard/save.js +++ b/superset/assets/cypress/integration/dashboard/save.js @@ -57,7 +57,7 @@ export default () => describe('save', () => { // should have box_plot chart const boxplotRequest = `/superset/explore_json/?form_data={"slice_id":${boxplotChartId}}`; - cy.route('POST', boxplotRequest).as('boxplotRequest'); + cy.route('GET', boxplotRequest).as('boxplotRequest'); cy.wait('@boxplotRequest'); cy.get('.grid-container .box_plot').should('be.exist'); diff --git a/superset/assets/cypress/integration/explore/control.test.js b/superset/assets/cypress/integration/explore/control.test.js index ba4636c47c76c..711ab783425cd 100644 --- a/superset/assets/cypress/integration/explore/control.test.js +++ b/superset/assets/cypress/integration/explore/control.test.js @@ -26,7 +26,8 @@ describe('Groupby', () => { cy.server(); cy.login(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); cy.visitChartByName('Num Births Trend'); cy.verifySliceSuccess({ waitAlias: '@getJson' }); @@ -36,7 +37,7 @@ describe('Groupby', () => { cy.get('.VirtualizedSelectFocusedOption').click(); }); cy.get('button.query').click(); - cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' }); + cy.verifySliceSuccess({ waitAlias: '@postJson', chartSelector: 'svg' }); }); }); @@ -44,7 +45,8 @@ describe('AdhocMetrics', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Clear metric and set simple adhoc metric', () => { @@ -74,7 +76,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', querySubstring: metricName, chartSelector: 'svg', }); @@ -105,7 +107,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', querySubstring: metric, chartSelector: 'svg', }); @@ -137,7 +139,7 @@ describe('AdhocMetrics', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -147,7 +149,8 @@ describe('AdhocFilters', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Set simple adhoc filter', () => { @@ -177,7 +180,7 @@ describe('AdhocFilters', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -206,7 +209,7 @@ describe('AdhocFilters', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); }); @@ -217,7 +220,8 @@ describe('Advanced analytics', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Create custom time compare', () => { @@ -240,7 +244,7 @@ describe('Advanced analytics', () => { }); cy.get('button.query').click(); - cy.wait('@getJson'); + cy.wait('@postJson'); cy.reload(); cy.verifySliceSuccess({ waitAlias: '@getJson', @@ -257,7 +261,8 @@ describe('Annotations', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Create formula annotation y-axis goal line', () => { @@ -280,7 +285,7 @@ describe('Annotations', () => { cy.get('button.query').click(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); @@ -292,7 +297,8 @@ describe('Time range filter', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Defaults to the correct tab for time_range params', () => { @@ -304,7 +310,7 @@ describe('Time range filter', () => { }; cy.visitChartByParams(JSON.stringify(formData)); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=time_range]').within(() => { cy.get('span.label').click(); diff --git a/superset/assets/cypress/integration/explore/link.test.js b/superset/assets/cypress/integration/explore/link.test.js index 024612f53b7d8..36b56ce5d5ac7 100644 --- a/superset/assets/cypress/integration/explore/link.test.js +++ b/superset/assets/cypress/integration/explore/link.test.js @@ -26,7 +26,8 @@ describe('Test explore links', () => { beforeEach(() => { cy.login(); cy.server(); - cy.route('POST', '/superset/explore_json/**').as('getJson'); + cy.route('GET', '/superset/explore_json/**').as('getJson'); + cy.route('POST', '/superset/explore_json/**').as('postJson'); }); it('Open and close view query modal', () => { @@ -35,7 +36,7 @@ describe('Test explore links', () => { cy.get('button#query').click(); cy.get('span').contains('View query').parent().click(); - cy.wait('@getJson').then(() => { + cy.wait('@postJson').then(() => { cy.get('code'); }); cy.get('.modal-header').within(() => { @@ -83,7 +84,7 @@ describe('Test explore links', () => { const newChartName = 'Test chart'; cy.visitChartByParams(JSON.stringify(formData)); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.url().then((url) => { cy.get('button[data-target="#save_modal"]').click(); cy.get('.modal-content').within(() => { @@ -109,7 +110,7 @@ describe('Test explore links', () => { cy.get('.modal-content').within(() => { cy.get('button#btn_modal_save').click(); }); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.request(`/chart/api/read?_flt_3_slice_name=${chartName}`).then((response) => { cy.request('DELETE', `/chart/api/delete/${response.body.pks[0]}`); }); From c0caac8bcde606086a4ca9067c97bb7076e5f0e2 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:12:17 -0700 Subject: [PATCH 77/86] Do POST request on new charts --- superset/assets/src/chart/Chart.jsx | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 88de2551f9f7f..8cb615361e2c5 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -68,12 +68,23 @@ class Chart extends React.PureComponent { } componentDidMount() { if (this.props.triggerQuery) { - this.props.actions.getSavedChart( - this.props.formData, - false, - this.props.timeout, - this.props.chartId, - ); + if (this.props.chartId > 0) { + // Load saved chart with a GET request + this.props.actions.getSavedChart( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } else { + // Create chart with POST request + this.props.actions.postChartFormData( + this.props.formData, + false, + this.props.timeout, + this.props.chartId, + ); + } } } From 4b9ffa737ef7053b158d3abede6350d7423e598f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:13:51 -0700 Subject: [PATCH 78/86] Set extra/adhoc filters only in GET requests --- superset/assets/src/chart/chartAction.js | 1 + superset/assets/src/explore/exploreUtils.js | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 2f1ce11dc750c..3909dadd85967 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -172,6 +172,7 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method) endpointType: 'json', force, allowDomainSharding: true, + method, }); const logStart = Logger.getTimestamp(); const controller = new AbortController(); diff --git a/superset/assets/src/explore/exploreUtils.js b/superset/assets/src/explore/exploreUtils.js index 7a04e8a1494a8..4785933385bb2 100644 --- a/superset/assets/src/explore/exploreUtils.js +++ b/superset/assets/src/explore/exploreUtils.js @@ -95,6 +95,7 @@ export function getExploreUrlAndPayload({ curUrl = null, requestParams = {}, allowDomainSharding = false, + method = 'POST', }) { if (!formData.datasource) { return null; @@ -118,12 +119,18 @@ export function getExploreUrlAndPayload({ // Building the querystring (search) part of the URI const search = uri.search(true); - if (formData.slice_id) { - search.form_data = safeStringify({ - slice_id: formData.slice_id, - extra_filters: formData.extra_filters, - adhoc_filters: formData.adhoc_filters, - }); + const { slice_id, extra_filters, adhoc_filters } = formData; + if (slice_id) { + const form_data = { slice_id }; + if (method === 'GET') { + if (extra_filters && extra_filters.length) { + form_data.extra_filters = extra_filters; + } + if (adhoc_filters && adhoc_filters.length) { + form_data.adhoc_filters = adhoc_filters; + } + } + search.form_data = safeStringify(form_data); } if (force) { search.force = 'true'; From b75e11ec1b8727ba4871a4432eb0bd2da5ae97ed Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:14:22 -0700 Subject: [PATCH 79/86] Raise if check_perms fails --- superset/utils/decorators.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 2fbc3f3048b3e..7db95f2354331 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -52,8 +52,10 @@ def etag_cache(max_age, check_perms=bool): def decorator(f): @wraps(f) def wrapper(*args, **kwargs): + # check if the user can access the resource + check_perms(*args, **kwargs) + try: - check_perms(request) # build the cache key from the function arguments and any other # additional GET arguments (like `form_data`, eg). key_args = list(args) From 96e3c9b42ac396331b134b6461041413cc6ca246 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:14:48 -0700 Subject: [PATCH 80/86] Refactor auth --- superset/views/core.py | 202 +++++++++++++--------------------------- superset/views/utils.py | 118 ++++++++++++++++++++++- 2 files changed, 182 insertions(+), 138 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index e7b8f8a58394b..b1af262b016b7 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -66,7 +66,7 @@ get_error_msg, handle_api_exception, json_error_response, json_success, SupersetFilter, SupersetModelView, YamlExportMixin, ) -from .utils import bootstrap_user_data +from .utils import bootstrap_user_data, get_datasource_info, get_form_data, get_viz config = app.config CACHE_DEFAULT_TIMEOUT = config.get('CACHE_DEFAULT_TIMEOUT', 0) @@ -102,11 +102,44 @@ def is_owner(obj, user): return obj and user in obj.owners -def check_perms(request): - """Check if user can access a cached response from explore_json""" - slice_id = json.loads(request.args.get('form_data'))['slice_id'] - slc = db.session.query(models.Slice).filter_by(id=slice_id).one() - security_manager.assert_datasource_permission(slc.get_viz().datasource) +def check_datasource_perms(self, datasource_type=None, datasource_id=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. + + """ + form_data = get_form_data()[0] + datasource_id, datasource_type = get_datasource_info( + datasource_id, datasource_type, form_data) + 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) + + +def check_slice_perms(self, slice_id): + """ + Check if user can access a cached response from slice_json. + + This function takes `self` since it must have the same signature as the + the decorated method. + + """ + form_data, slc = get_form_data(slice_id, use_slice_data=True) + datasource_type = slc.datasource.type + datasource_id = slc.datasource.id + 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) class DatabaseFilter(SupersetFilter): @@ -1021,90 +1054,10 @@ def clean_fulfilled_requests(session): session.commit() return redirect('/accessrequestsmodelview/list/') - def get_form_data(self, slice_id=None, use_slice_data=False): - form_data = {} - post_data = request.form.get('form_data') - request_args_data = request.args.get('form_data') - # Supporting POST - if post_data: - form_data.update(json.loads(post_data)) - # request params can overwrite post body - if request_args_data: - form_data.update(json.loads(request_args_data)) - - url_id = request.args.get('r') - if url_id: - saved_url = db.session.query(models.Url).filter_by(id=url_id).first() - if saved_url: - url_str = parse.unquote_plus( - saved_url.url.split('?')[1][10:], encoding='utf-8', errors=None) - url_form_data = json.loads(url_str) - # allow form_date in request override saved url - url_form_data.update(form_data) - form_data = url_form_data - - if request.args.get('viz_type'): - # Converting old URLs - form_data = cast_form_data(form_data) - - form_data = { - k: v - for k, v in form_data.items() - if k not in FORM_DATA_KEY_BLACKLIST - } - - # When a slice_id is present, load from DB and override - # the form_data from the DB with the other form_data provided - slice_id = form_data.get('slice_id') or slice_id - slc = None - - # Check if form data only contains slice_id and additional filters - valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] - valid_slice_id = all(key in valid_keys for key in form_data) - - # Include the slice_form_data if request from explore or slice calls - # or if form_data only contains slice_id and additional filters - if slice_id and (use_slice_data or valid_slice_id): - slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() - if slc: - slice_form_data = slc.form_data.copy() - slice_form_data.update(form_data) - form_data = slice_form_data - - update_time_range(form_data) - - return form_data, slc - - def get_viz( - self, - slice_id=None, - form_data=None, - datasource_type=None, - datasource_id=None, - force=False, - ): - if slice_id: - slc = ( - db.session.query(models.Slice) - .filter_by(id=slice_id) - .one() - ) - return slc.get_viz() - else: - viz_type = form_data.get('viz_type', 'table') - datasource = ConnectorRegistry.get_datasource( - datasource_type, datasource_id, db.session) - viz_obj = viz.viz_types[viz_type]( - datasource, - form_data=form_data, - force=force, - ) - return viz_obj - @has_access @expose('/slice//') def slice(self, slice_id): - form_data, slc = self.get_form_data(slice_id, use_slice_data=True) + form_data, slc = get_form_data(slice_id, use_slice_data=True) if not slc: abort(404) endpoint = '/superset/explore/?form_data={}'.format( @@ -1148,18 +1101,7 @@ def get_samples(self, viz_obj): }) def generate_json( - self, datasource_type, datasource_id, form_data, - csv=False, query=False, force=False, results=False, - samples=False, - ): - viz_obj = self.get_viz( - datasource_type=datasource_type, - datasource_id=datasource_id, - form_data=form_data, - force=force, - ) - security_manager.assert_datasource_permission(viz_obj.datasource) - + self, viz_obj, csv=False, query=False, results=False, samples=False): if csv: return CsvResponse( viz_obj.get_csv(), @@ -1183,21 +1125,25 @@ def generate_json( @api @has_access_api @expose('/slice_json/') + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_slice_perms) def slice_json(self, slice_id): - form_data, slc = self.get_form_data(slice_id, use_slice_data=True) + form_data, slc = get_form_data(slice_id, use_slice_data=True) datasource_type = slc.datasource.type datasource_id = slc.datasource.id - - return self.generate_json(datasource_type=datasource_type, - datasource_id=datasource_id, - form_data=form_data) + viz_obj = get_viz( + datasource_type=datasource_type, + datasource_id=datasource_id, + form_data=form_data, + force=False, + ) + return self.generate_json(viz_obj) @log_this @api @has_access_api @expose('/annotation_json/') def annotation_json(self, layer_id): - form_data = self.get_form_data()[0] + form_data = get_form_data()[0] form_data['layer_id'] = layer_id form_data['filters'] = [{'col': 'layer_id', 'op': '==', @@ -1217,7 +1163,7 @@ def annotation_json(self, layer_id): @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) - @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_perms) + @etag_cache(CACHE_DEFAULT_TIMEOUT, check_perms=check_datasource_perms) def explore_json(self, datasource_type=None, datasource_id=None): """Serves all request that GET or POST form_data @@ -1234,18 +1180,21 @@ def explore_json(self, datasource_type=None, datasource_id=None): samples = request.args.get('samples') == 'true' force = request.args.get('force') == 'true' - form_data = self.get_form_data()[0] - datasource_id, datasource_type = self.datasource_info( + form_data = get_form_data()[0] + datasource_id, datasource_type = get_datasource_info( datasource_id, datasource_type, form_data) - - return self.generate_json( + viz_obj = get_viz( datasource_type=datasource_type, datasource_id=datasource_id, form_data=form_data, + force=force, + ) + + return self.generate_json( + viz_obj, csv=csv, query=query, results=results, - force=force, samples=samples, ) @@ -1271,34 +1220,15 @@ def explorev2(self, datasource_type, datasource_id): datasource_id=datasource_id, **request.args)) - @staticmethod - def datasource_info(datasource_id, datasource_type, form_data): - """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""" - 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 not datasource_id: - raise Exception( - 'The datasource associated with this chart no longer exists') - datasource_id = int(datasource_id) - return datasource_id, datasource_type - @log_this @has_access @expose('/explore///', methods=['GET', 'POST']) @expose('/explore/', methods=['GET', 'POST']) def explore(self, datasource_type=None, datasource_id=None): user_id = g.user.get_id() if g.user else None - form_data, slc = self.get_form_data(use_slice_data=True) + form_data, slc = get_form_data(use_slice_data=True) - datasource_id, datasource_type = self.datasource_info( + datasource_id, datasource_type = get_datasource_info( datasource_id, datasource_type, form_data) error_redirect = '/chart/list/' @@ -1424,7 +1354,7 @@ def save_or_overwrite_slice( """Save or overwrite a slice""" slice_name = args.get('slice_name') action = args.get('action') - form_data, unused_slc = self.get_form_data() + form_data = get_form_data()[0] if action in ('saveas'): if 'slice_id' in form_data: @@ -2116,8 +2046,8 @@ def warm_up_cache(self): for slc in slices: try: - form_data = self.get_form_data(slc.id, use_slice_data=True)[0] - obj = self.get_viz( + form_data = get_form_data(slc.id, use_slice_data=True)[0] + obj = get_viz( datasource_type=slc.datasource.type, datasource_id=slc.datasource.id, form_data=form_data, @@ -2885,7 +2815,7 @@ def slice_query(self, slice_id): This method exposes an API endpoint to get the database query string for this slice """ - viz_obj = self.get_viz(slice_id) + viz_obj = get_viz(slice_id) security_manager.assert_datasource_permission(viz_obj.datasource) return self.get_query_string_response(viz_obj) diff --git a/superset/views/utils.py b/superset/views/utils.py index eb68316baaffb..4318141e36014 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -16,11 +16,25 @@ # under the License. # pylint: disable=C,R,W from collections import defaultdict +from urllib import parse -from flask import g +from flask import g, request from flask_appbuilder.security.sqla import models as ab_models +import simplejson as json -from superset import db +from superset import app, db, viz +from superset.connectors.connector_registry import ConnectorRegistry +from superset.legacy import cast_form_data, update_time_range +import superset.models.core as models + + +FORM_DATA_KEY_BLACKLIST = [] +if not app.config.get('ENABLE_JAVASCRIPT_CONTROLS'): + FORM_DATA_KEY_BLACKLIST = [ + 'js_tooltip', + 'js_onclick_href', + 'js_data_mutator', + ] def bootstrap_user_data(username=None, include_perms=False): @@ -74,3 +88,103 @@ def get_permissions(user): ] return roles, permissions + + +def get_viz( + slice_id=None, + form_data=None, + datasource_type=None, + datasource_id=None, + force=False, +): + if slice_id: + slc = ( + db.session.query(models.Slice) + .filter_by(id=slice_id) + .one() + ) + return slc.get_viz() + else: + viz_type = form_data.get('viz_type', 'table') + datasource = ConnectorRegistry.get_datasource( + datasource_type, datasource_id, db.session) + viz_obj = viz.viz_types[viz_type]( + datasource, + form_data=form_data, + force=force, + ) + return viz_obj + + +def get_form_data(slice_id=None, use_slice_data=False): + form_data = {} + post_data = request.form.get('form_data') + request_args_data = request.args.get('form_data') + # Supporting POST + if post_data: + form_data.update(json.loads(post_data)) + # request params can overwrite post body + if request_args_data: + form_data.update(json.loads(request_args_data)) + + url_id = request.args.get('r') + if url_id: + saved_url = db.session.query(models.Url).filter_by(id=url_id).first() + if saved_url: + url_str = parse.unquote_plus( + saved_url.url.split('?')[1][10:], encoding='utf-8', errors=None) + url_form_data = json.loads(url_str) + # allow form_date in request override saved url + url_form_data.update(form_data) + form_data = url_form_data + + if request.args.get('viz_type'): + # Converting old URLs + form_data = cast_form_data(form_data) + + form_data = { + k: v + for k, v in form_data.items() + if k not in FORM_DATA_KEY_BLACKLIST + } + + # When a slice_id is present, load from DB and override + # the form_data from the DB with the other form_data provided + slice_id = form_data.get('slice_id') or slice_id + slc = None + + # Check if form data only contains slice_id and additional filters + valid_keys = ['slice_id', 'extra_filters', 'adhoc_filters'] + valid_slice_id = all(key in valid_keys for key in form_data) + + # Include the slice_form_data if request from explore or slice calls + # or if form_data only contains slice_id and additional filters + if slice_id and (use_slice_data or valid_slice_id): + slc = db.session.query(models.Slice).filter_by(id=slice_id).one_or_none() + if slc: + slice_form_data = slc.form_data.copy() + slice_form_data.update(form_data) + form_data = slice_form_data + + update_time_range(form_data) + + return form_data, slc + + +def get_datasource_info(datasource_id, datasource_type, form_data): + """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""" + 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 not datasource_id: + raise Exception( + 'The datasource associated with this chart no longer exists') + datasource_id = int(datasource_id) + return datasource_id, datasource_type From a1fdb44f30d395140db6e5dbafbc440b8efa65e1 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 22 Mar 2019 11:38:30 -0700 Subject: [PATCH 81/86] Fix flake8 --- superset/views/core.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index b1af262b016b7..61fde62c2895d 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -50,7 +50,6 @@ from superset.exceptions import SupersetException from superset.forms import CsvToDatabaseForm from superset.jinja_context import get_template_processor -from superset.legacy import cast_form_data, update_time_range import superset.models.core as models from superset.models.sql_lab import Query from superset.models.user_attributes import UserAttribute @@ -105,10 +104,10 @@ def is_owner(obj, user): def check_datasource_perms(self, datasource_type=None, datasource_id=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. - + """ form_data = get_form_data()[0] datasource_id, datasource_type = get_datasource_info( @@ -125,10 +124,10 @@ def check_datasource_perms(self, datasource_type=None, datasource_id=None): def check_slice_perms(self, slice_id): """ Check if user can access a cached response from slice_json. - + This function takes `self` since it must have the same signature as the the decorated method. - + """ form_data, slc = get_form_data(slice_id, use_slice_data=True) datasource_type = slc.datasource.type @@ -1101,7 +1100,7 @@ def get_samples(self, viz_obj): }) def generate_json( - self, viz_obj, csv=False, query=False, results=False, samples=False): + self, viz_obj, csv=False, query=False, results=False, samples=False): if csv: return CsvResponse( viz_obj.get_csv(), From d0bff86c73c613e500984ad79309083aa896949b Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 12:07:08 -0700 Subject: [PATCH 82/86] Fix js unit tests --- .../spec/javascripts/chart/chartActions_spec.js | 14 +++++++------- .../dashboard/components/Dashboard_spec.jsx | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/superset/assets/spec/javascripts/chart/chartActions_spec.js b/superset/assets/spec/javascripts/chart/chartActions_spec.js index 09f618c6c7d6b..a4a832632e3de 100644 --- a/superset/assets/spec/javascripts/chart/chartActions_spec.js +++ b/superset/assets/spec/javascripts/chart/chartActions_spec.js @@ -51,7 +51,7 @@ describe('chart actions', () => { }); it('should dispatch CHART_UPDATE_STARTED action before the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success @@ -64,7 +64,7 @@ describe('chart actions', () => { }); it('should dispatch TRIGGER_QUERY action with the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -76,7 +76,7 @@ describe('chart actions', () => { }); it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -88,7 +88,7 @@ describe('chart actions', () => { }); it('should dispatch logEvent async action', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -104,7 +104,7 @@ describe('chart actions', () => { }); it('should dispatch CHART_UPDATE_SUCCEEDED action upon success', () => { - const actionThunk = actions.runQuery({}); + const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); @@ -120,7 +120,7 @@ describe('chart actions', () => { fetchMock.post(MOCK_URL, () => unresolvingPromise, { overwriteRoutes: true }); const timeoutInSec = 1 / 1000; - const actionThunk = actions.runQuery({}, false, timeoutInSec); + const actionThunk = actions.postChartFormData({}, false, timeoutInSec); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, fail @@ -136,7 +136,7 @@ describe('chart actions', () => { fetchMock.post(MOCK_URL, { throws: { statusText: 'misc error' } }, { overwriteRoutes: true }); const timeoutInSec = 1 / 1000; - const actionThunk = actions.runQuery({}, false, timeoutInSec); + const actionThunk = actions.postChartFormData({}, false, timeoutInSec); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, fail diff --git a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx index 523b835d475bf..de637cd2ada9a 100644 --- a/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx +++ b/superset/assets/spec/javascripts/dashboard/components/Dashboard_spec.jsx @@ -39,7 +39,7 @@ describe('Dashboard', () => { actions: { addSliceToDashboard() {}, removeSliceFromDashboard() {}, - runQuery() {}, + postChartFormData() {}, logEvent() {}, }, initMessages: [], @@ -82,15 +82,15 @@ describe('Dashboard', () => { }, }; - it('should call runQuery for all non-exempt slices', () => { + it('should call postChartFormData for all non-exempt slices', () => { const wrapper = setup({ charts: overrideCharts, slices: overrideSlices }); - const spy = sinon.spy(props.actions, 'runQuery'); + const spy = sinon.spy(props.actions, 'postChartFormData'); wrapper.instance().refreshExcept('1001'); spy.restore(); expect(spy.callCount).toBe(Object.keys(overrideCharts).length - 1); }); - it('should not call runQuery for filter_immune_slices', () => { + it('should not call postChartFormData for filter_immune_slices', () => { const wrapper = setup({ charts: overrideCharts, dashboardInfo: { @@ -103,7 +103,7 @@ describe('Dashboard', () => { }, }, }); - const spy = sinon.spy(props.actions, 'runQuery'); + const spy = sinon.spy(props.actions, 'postChartFormData'); wrapper.instance().refreshExcept(); spy.restore(); expect(spy.callCount).toBe(0); From 057b953886dfbbf4e5d11104eb1222e5c8c7133c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 12:23:22 -0700 Subject: [PATCH 83/86] Fix js unit tests that fail in lyftga --- .../assets/spec/javascripts/components/AsyncSelect_spec.jsx | 2 +- .../assets/spec/javascripts/welcome/DashboardTable_spec.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx index 6918e1b94ccc7..dd466ceb6b47e 100644 --- a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx +++ b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx @@ -76,7 +76,7 @@ describe('AsyncSelect', () => { ); setTimeout(() => { - expect(fetchMock.calls(dataGlob)).toHaveLength(1); + expect(fetchMock.calls(dataGlob)).toHaveLength(3); expect(onChangeSpy.mock.calls).toHaveLength(0); done(); }); diff --git a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx index e989f9a207aea..3cbb1075f29d6 100644 --- a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx +++ b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx @@ -54,7 +54,7 @@ describe('DashboardTable', () => { const wrapper = setup(); setTimeout(() => { - expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(1); + expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(2); // there's a delay between response and updating state, so manually set it // rather than adding a timeout which could introduce flakiness wrapper.setState({ dashaboards: mockDashboards }); From 7da01578b9844ffd273d589b5291ad3c85a7eb56 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 25 Mar 2019 13:22:06 -0700 Subject: [PATCH 84/86] Fix js --- .../assets/spec/javascripts/components/AsyncSelect_spec.jsx | 2 +- .../assets/spec/javascripts/welcome/DashboardTable_spec.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx index dd466ceb6b47e..6918e1b94ccc7 100644 --- a/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx +++ b/superset/assets/spec/javascripts/components/AsyncSelect_spec.jsx @@ -76,7 +76,7 @@ describe('AsyncSelect', () => { ); setTimeout(() => { - expect(fetchMock.calls(dataGlob)).toHaveLength(3); + expect(fetchMock.calls(dataGlob)).toHaveLength(1); expect(onChangeSpy.mock.calls).toHaveLength(0); done(); }); diff --git a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx index 3cbb1075f29d6..e989f9a207aea 100644 --- a/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx +++ b/superset/assets/spec/javascripts/welcome/DashboardTable_spec.jsx @@ -54,7 +54,7 @@ describe('DashboardTable', () => { const wrapper = setup(); setTimeout(() => { - expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(2); + expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(1); // there's a delay between response and updating state, so manually set it // rather than adding a timeout which could introduce flakiness wrapper.setState({ dashaboards: mockDashboards }); From a364194cf03443c84aac8db54be61d06d0428871 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 2 Apr 2019 11:36:47 -0700 Subject: [PATCH 85/86] Fix bad merge --- .../assets/src/components/FilterableTable/FilterableTable.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/superset/assets/src/components/FilterableTable/FilterableTable.jsx b/superset/assets/src/components/FilterableTable/FilterableTable.jsx index 55cfcbe48286f..72a22c7ba20c3 100644 --- a/superset/assets/src/components/FilterableTable/FilterableTable.jsx +++ b/superset/assets/src/components/FilterableTable/FilterableTable.jsx @@ -34,8 +34,6 @@ function getTextWidth(text, font = '12px Roboto') { const SCROLL_BAR_HEIGHT = 15; -const SCROLL_BAR_HEIGHT = 15; - const propTypes = { orderedColumnKeys: PropTypes.array.isRequired, data: PropTypes.array.isRequired, From 5e418ed71aed5f239a2bc5c4f3c0841861e8ad1c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 2 Apr 2019 14:23:50 -0700 Subject: [PATCH 86/86] Use far future when max_age=0 --- superset/utils/decorators.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 7db95f2354331..23b15a5d66ba4 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -25,6 +25,12 @@ from superset.utils.dates import now_as_float +# If a user sets `max_age` to 0, for long the browser should cache the +# resource? Flask-Caching will cache forever, but for the HTTP header we need +# to specify a "far future" date. +FAR_FUTURE = 365 * 24 * 60 * 60 # 1 year in seconds + + @contextmanager def stats_timing(stats_key, stats_logger): """Provide a transactional scope around a series of operations.""" @@ -73,7 +79,8 @@ def wrapper(*args, **kwargs): response = f(*args, **kwargs) response.cache_control.public = True response.last_modified = datetime.utcnow() - response.expires = response.last_modified + timedelta(seconds=max_age) + expiration = max_age if max_age != 0 else FAR_FUTURE + response.expires = response.last_modified + timedelta(seconds=expiration) response.add_etag() try: cache.set(cache_key, response, timeout=max_age)