You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/05/26 13:29:02 UTC
[superset] branch 1.5 updated (94d4179045 -> 8089afce1e)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
from 94d4179045 update changelog entries
new 2bd89d1705 fix: Alpha should not be able to edit datasets that they don't own (#19854)
new 9ca53b8905 fix(sqla): replace custom dttm type with literal_column (#19917)
new 8becd3e080 fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp (#18873)
new 4e44f4cc11 fix(plugin-chart-echarts): support adhoc x-axis (#20055)
new bda9044bfd fix: Athena timestamp literal format (#19970)
new 97c6af196e fix(generic-axes): apply contribution before flatten (#20077)
new c5e2809c2c fix(presto,trino): use correct literal dttm separator (#20123)
new 562e3a769b fix: Allow dataset owners to see their datasets (#20135)
new d652e0cab2 fix: "Week Staring Monday" time grain for BigQuery (#20091)
new 8089afce1e fix(temporary-cache): when user is anonymous (#20181)
The 10 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../src/operators/pivotOperator.ts | 4 +-
.../src/operators/prophetOperator.ts | 9 ++-
.../src/operators/timeComparePivotOperator.ts | 3 +-
.../src/shared-controls/index.tsx | 6 ++
.../test/utils/operators/pivotOperator.test.ts | 32 +++++++++++
.../test/utils/operators/prophetOperator.test.ts | 33 +++++++++++
.../operators/timeComparePivotOperator.test.ts | 41 ++++++++++++++
.../src/Timeseries/buildQuery.ts | 6 +-
.../src/Timeseries/transformProps.ts | 4 +-
.../DatasourcePanel/DatasourcePanel.test.tsx | 17 ++++++
.../explore/components/DatasourcePanel/index.tsx | 2 +
.../DatasourceControl/DatasourceControl.test.jsx | 13 +++++
.../DatasourceControl/DatasourceControl.test.tsx | 11 ++++
.../controls/DatasourceControl/index.jsx | 25 ++++++++-
.../views/CRUD/data/dataset/DatasetList.test.jsx | 1 +
.../src/views/CRUD/data/dataset/DatasetList.tsx | 40 +++++++++++--
superset/connectors/base/models.py | 31 ++++++++---
superset/connectors/druid/models.py | 2 +-
superset/connectors/sqla/models.py | 20 ++++---
.../dashboards/filter_state/commands/create.py | 17 +++---
.../dashboards/filter_state/commands/delete.py | 22 ++++----
superset/dashboards/filter_state/commands/get.py | 4 +-
.../dashboards/filter_state/commands/update.py | 45 +++++++--------
.../filter_state/commands/utils.py} | 28 ++++++----
superset/db_engine_specs/athena.py | 6 +-
superset/db_engine_specs/base.py | 9 +--
superset/db_engine_specs/bigquery.py | 1 +
superset/db_engine_specs/presto.py | 6 +-
superset/db_engine_specs/trino.py | 4 +-
superset/explore/form_data/api.py | 45 ++++-----------
superset/explore/form_data/commands/create.py | 6 +-
superset/explore/form_data/commands/delete.py | 5 +-
superset/explore/form_data/commands/get.py | 2 +-
superset/explore/form_data/commands/state.py | 2 +-
superset/explore/form_data/commands/update.py | 10 ++--
.../form_data/commands/{parameters.py => utils.py} | 30 +++++++---
superset/explore/utils.py | 8 +--
superset/key_value/utils.py | 7 ++-
superset/models/sql_types/base.py | 65 ----------------------
superset/result_set.py | 5 +-
superset/temporary_cache/api.py | 51 ++++-------------
superset/temporary_cache/commands/entry.py | 4 +-
superset/temporary_cache/commands/exceptions.py | 4 ++
superset/utils/core.py | 4 +-
superset/views/base.py | 9 +++
superset/views/datasource/views.py | 34 ++++-------
tests/integration_tests/datasets/api_tests.py | 21 +++++++
tests/integration_tests/datasource_tests.py | 8 +++
.../db_engine_specs/presto_tests.py | 15 -----
.../db_engine_specs/trino_tests.py | 13 -----
tests/integration_tests/model_tests.py | 24 +-------
tests/integration_tests/utils_tests.py | 5 ++
tests/unit_tests/dataframe_test.py | 52 +++++++++++++++++
tests/unit_tests/db_engine_specs/test_athena.py | 9 +--
tests/unit_tests/db_engine_specs/test_presto.py | 8 +--
tests/unit_tests/db_engine_specs/test_trino.py | 8 +--
tests/unit_tests/explore/utils_test.py | 8 +--
57 files changed, 540 insertions(+), 364 deletions(-)
copy superset/{datasets/columns/commands/exceptions.py => dashboards/filter_state/commands/utils.py} (58%)
copy superset/explore/form_data/commands/{parameters.py => utils.py} (51%)
delete mode 100644 superset/models/sql_types/base.py
[superset] 03/10: fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp (#18873)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8becd3e0802d692a9be8d3dc8cb4c32b036deccd
Author: Yeachan Park <ye...@gmail.com>
AuthorDate: Wed May 4 08:47:12 2022 +0200
fix(sqllab/charts): casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp (#18873)
* fix casting from timestamp[us] to timestamp[ns] would result in out of bounds timestamp from sqllab and charts
* Add unittests
* Lint changes and parameter variable rename
* Fix linting
(cherry picked from commit 8b72354654a8d1eec02510ca011644d3856fa09e)
---
superset/result_set.py | 5 +++-
superset/utils/core.py | 4 +--
tests/integration_tests/utils_tests.py | 5 ++++
tests/unit_tests/dataframe_test.py | 52 ++++++++++++++++++++++++++++++++++
4 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/superset/result_set.py b/superset/result_set.py
index 82b0a31393..008699d683 100644
--- a/superset/result_set.py
+++ b/superset/result_set.py
@@ -174,7 +174,10 @@ class SupersetResultSet:
@staticmethod
def convert_table_to_df(table: pa.Table) -> pd.DataFrame:
- return table.to_pandas(integer_object_nulls=True)
+ try:
+ return table.to_pandas(integer_object_nulls=True)
+ except pa.lib.ArrowInvalid:
+ return table.to_pandas(integer_object_nulls=True, timestamp_as_object=True)
@staticmethod
def first_nonempty(items: List[Any]) -> Any:
diff --git a/superset/utils/core.py b/superset/utils/core.py
index aafe18a1e1..4177e01841 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -1762,14 +1762,14 @@ def normalize_dttm_col(
# Column is formatted as a numeric value
unit = timestamp_format.replace("epoch_", "")
df[DTTM_ALIAS] = pd.to_datetime(
- dttm_col, utc=False, unit=unit, origin="unix"
+ dttm_col, utc=False, unit=unit, origin="unix", errors="coerce"
)
else:
# Column has already been formatted as a timestamp.
df[DTTM_ALIAS] = dttm_col.apply(pd.Timestamp)
else:
df[DTTM_ALIAS] = pd.to_datetime(
- df[DTTM_ALIAS], utc=False, format=timestamp_format
+ df[DTTM_ALIAS], utc=False, format=timestamp_format, errors="coerce"
)
if offset:
df[DTTM_ALIAS] += timedelta(hours=offset)
diff --git a/tests/integration_tests/utils_tests.py b/tests/integration_tests/utils_tests.py
index 012bb96ca9..0e145eab7f 100644
--- a/tests/integration_tests/utils_tests.py
+++ b/tests/integration_tests/utils_tests.py
@@ -1201,3 +1201,8 @@ class TestUtils(SupersetTestCase):
# test numeric epoch_ms format
df = pd.DataFrame([{"__timestamp": ts.timestamp() * 1000, "a": 1}])
assert normalize_col(df, "epoch_ms", 0, None)[DTTM_ALIAS][0] == ts
+
+ # test that out of bounds timestamps are coerced to None instead of
+ # erroring out
+ df = pd.DataFrame([{"__timestamp": "1677-09-21 00:00:00", "a": 1}])
+ assert pd.isnull(normalize_col(df, None, 0, None)[DTTM_ALIAS][0])
diff --git a/tests/unit_tests/dataframe_test.py b/tests/unit_tests/dataframe_test.py
index 79625cffe6..8785ee1d7b 100644
--- a/tests/unit_tests/dataframe_test.py
+++ b/tests/unit_tests/dataframe_test.py
@@ -15,6 +15,11 @@
# specific language governing permissions and limitations
# under the License.
# pylint: disable=unused-argument, import-outside-toplevel
+from datetime import datetime
+
+import pytest
+from pandas import Timestamp
+
from superset.dataframe import df_to_records
from superset.superset_typing import DbapiDescription
@@ -53,3 +58,50 @@ def test_js_max_int(app_context: None) -> None:
{"a": 1, "b": "1239162456494753670", "c": "c1"},
{"a": 2, "b": 100, "c": "c2"},
]
+
+
+@pytest.mark.parametrize(
+ "input_, expected",
+ [
+ pytest.param(
+ [
+ (datetime.strptime("1677-09-22 00:12:43", "%Y-%m-%d %H:%M:%S"), 1),
+ (datetime.strptime("2262-04-11 23:47:17", "%Y-%m-%d %H:%M:%S"), 2),
+ ],
+ [
+ {
+ "a": datetime.strptime("1677-09-22 00:12:43", "%Y-%m-%d %H:%M:%S"),
+ "b": 1,
+ },
+ {
+ "a": datetime.strptime("2262-04-11 23:47:17", "%Y-%m-%d %H:%M:%S"),
+ "b": 2,
+ },
+ ],
+ id="timestamp conversion fail",
+ ),
+ pytest.param(
+ [
+ (datetime.strptime("1677-09-22 00:12:44", "%Y-%m-%d %H:%M:%S"), 1),
+ (datetime.strptime("2262-04-11 23:47:16", "%Y-%m-%d %H:%M:%S"), 2),
+ ],
+ [
+ {"a": Timestamp("1677-09-22 00:12:44"), "b": 1},
+ {"a": Timestamp("2262-04-11 23:47:16"), "b": 2},
+ ],
+ id="timestamp conversion success",
+ ),
+ ],
+)
+def test_max_pandas_timestamp(input_, expected) -> None:
+ from superset.db_engine_specs import BaseEngineSpec
+ from superset.result_set import SupersetResultSet
+
+ cursor_descr: DbapiDescription = [
+ ("a", "datetime", None, None, None, None, False),
+ ("b", "int", None, None, None, None, False),
+ ]
+ results = SupersetResultSet(input_, cursor_descr, BaseEngineSpec)
+ df = results.to_pandas_df()
+
+ assert df_to_records(df) == expected
[superset] 06/10: fix(generic-axes): apply contribution before flatten (#20077)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 97c6af196e919cd2d7c1a7ebe31fb34325261a3f
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Mon May 16 16:26:31 2022 +0300
fix(generic-axes): apply contribution before flatten (#20077)
(cherry picked from commit d5802f78964a5027184ff9e7f6b78c14b04fd988)
---
.../plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
index c2f603bfc4..24c96b9202 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
@@ -96,11 +96,11 @@ export default function buildQuery(formData: QueryFormData) {
...baseQueryObject,
...{ is_timeseries },
}),
- flattenOperator(formData, baseQueryObject),
- // todo: move contribution and prophet before flatten
contributionOperator(formData, baseQueryObject),
+ flattenOperator(formData, baseQueryObject),
+ // todo: move prophet before flatten
prophetOperator(formData, baseQueryObject),
- ],
+ ].filter(op => op),
},
];
});
[superset] 09/10: fix: "Week Staring Monday" time grain for BigQuery (#20091)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d652e0cab2e9d0d5145dfe7db6fa7c0c8ef29de2
Author: Ramunas Balukonis <95...@users.noreply.github.com>
AuthorDate: Thu May 26 12:31:32 2022 +0300
fix: "Week Staring Monday" time grain for BigQuery (#20091)
* Week Staring From Monday added for BQ
* lint fix
(cherry picked from commit 694f75d37657a554831fe9747e5baef36339ed88)
---
superset/db_engine_specs/bigquery.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py
index 2c9f81b1bd..cd040262f6 100644
--- a/superset/db_engine_specs/bigquery.py
+++ b/superset/db_engine_specs/bigquery.py
@@ -140,6 +140,7 @@ class BigQueryEngineSpec(BaseEngineSpec):
"PT1H": "{func}({col}, HOUR)",
"P1D": "{func}({col}, DAY)",
"P1W": "{func}({col}, WEEK)",
+ "1969-12-29T00:00:00Z/P1W": "{func}({col}, ISOWEEK)",
"P1M": "{func}({col}, MONTH)",
"P3M": "{func}({col}, QUARTER)",
"P1Y": "{func}({col}, YEAR)",
[superset] 04/10: fix(plugin-chart-echarts): support adhoc x-axis (#20055)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4e44f4cc11a67a2ed5c74dc4f20f631526211201
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Fri May 13 16:29:51 2022 +0300
fix(plugin-chart-echarts): support adhoc x-axis (#20055)
* fix(plugin-chart-echarts): support adhoc x-axis
* simplify code
(cherry picked from commit b53daa91ecf0e82fe219b498e907d0c3f3ca9ccb)
---
.../src/operators/pivotOperator.ts | 4 ++-
.../src/operators/prophetOperator.ts | 9 +++--
.../src/operators/timeComparePivotOperator.ts | 3 +-
.../test/utils/operators/pivotOperator.test.ts | 32 +++++++++++++++++
.../test/utils/operators/prophetOperator.test.ts | 33 +++++++++++++++++
.../operators/timeComparePivotOperator.test.ts | 41 ++++++++++++++++++++++
.../src/Timeseries/transformProps.ts | 4 ++-
7 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
index a5bf20da88..f8e9f025b0 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
@@ -31,11 +31,13 @@ export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
) => {
const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
const { x_axis: xAxis } = formData;
+
if ((xAxis || queryObject.is_timeseries) && metricLabels.length) {
+ const index = [getColumnLabel(xAxis || DTTM_ALIAS)];
return {
operation: 'pivot',
options: {
- index: [xAxis || DTTM_ALIAS],
+ index,
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
// Create 'dummy' mean aggregates to assign cell values in pivot table
// use the 'mean' aggregates to avoid drop NaN. PR: https://github.com/apache-superset/superset-ui/pull/1231
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
index 297d84ea1b..a55c8d3d9e 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
@@ -16,13 +16,18 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
-import { DTTM_ALIAS, PostProcessingProphet } from '@superset-ui/core';
+import {
+ DTTM_ALIAS,
+ getColumnLabel,
+ PostProcessingProphet,
+} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
formData,
queryObject,
) => {
+ const index = getColumnLabel(formData.x_axis || DTTM_ALIAS);
if (formData.forecastEnabled) {
return {
operation: 'prophet',
@@ -33,7 +38,7 @@ export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
yearly_seasonality: formData.forecastSeasonalityYearly,
weekly_seasonality: formData.forecastSeasonalityWeekly,
daily_seasonality: formData.forecastSeasonalityDaily,
- index: formData.x_axis || DTTM_ALIAS,
+ index,
},
};
}
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
index 44a1825ff8..be1c1f936d 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
@@ -39,11 +39,12 @@ export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot
{ operator: 'mean' as NumpyFunction },
]),
);
+ const index = [getColumnLabel(formData.x_axis || DTTM_ALIAS)];
return {
operation: 'pivot',
options: {
- index: [formData.x_axis || DTTM_ALIAS],
+ index,
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
drop_missing_columns: false,
flatten_columns: false,
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/pivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/pivotOperator.test.ts
index b75385a42c..9787136cbc 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/pivotOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/pivotOperator.test.ts
@@ -136,3 +136,35 @@ test('pivot by x_axis with groupby', () => {
},
});
});
+
+test('pivot by adhoc x_axis', () => {
+ expect(
+ pivotOperator(
+ {
+ ...formData,
+ x_axis: {
+ label: 'my_case_expr',
+ expressionType: 'SQL',
+ expression: 'case when a = 1 then 1 else 0 end',
+ },
+ },
+ {
+ ...queryObject,
+ columns: ['foo', 'bar'],
+ },
+ ),
+ ).toEqual({
+ operation: 'pivot',
+ options: {
+ index: ['my_case_expr'],
+ columns: ['foo', 'bar'],
+ aggregates: {
+ 'count(*)': { operator: 'mean' },
+ 'sum(val)': { operator: 'mean' },
+ },
+ drop_missing_columns: false,
+ flatten_columns: false,
+ reset_index: false,
+ },
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/prophetOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/prophetOperator.test.ts
index 78d4bc9765..48ced3ba1f 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/prophetOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/prophetOperator.test.ts
@@ -98,3 +98,36 @@ test('should do prophetOperator over named column', () => {
},
});
});
+
+test('should do prophetOperator over adhoc column', () => {
+ expect(
+ prophetOperator(
+ {
+ ...formData,
+ x_axis: {
+ label: 'my_case_expr',
+ expressionType: 'SQL',
+ expression: 'case when a = 1 then 1 else 0 end',
+ },
+ forecastEnabled: true,
+ forecastPeriods: '3',
+ forecastInterval: '5',
+ forecastSeasonalityYearly: true,
+ forecastSeasonalityWeekly: false,
+ forecastSeasonalityDaily: false,
+ },
+ queryObject,
+ ),
+ ).toEqual({
+ operation: 'prophet',
+ options: {
+ time_grain: 'P1Y',
+ periods: 3.0,
+ confidence_interval: 5.0,
+ yearly_seasonality: true,
+ weekly_seasonality: false,
+ daily_seasonality: false,
+ index: 'my_case_expr',
+ },
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/timeComparePivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/timeComparePivotOperator.test.ts
index fcf8ea6367..b12f6ebe30 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/timeComparePivotOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/operators/timeComparePivotOperator.test.ts
@@ -135,3 +135,44 @@ test('should pivot on x-axis', () => {
},
});
});
+
+test('should pivot on adhoc x-axis', () => {
+ expect(
+ timeComparePivotOperator(
+ {
+ ...formData,
+ comparison_type: 'values',
+ time_compare: ['1 year ago', '1 year later'],
+ x_axis: {
+ label: 'my_case_expr',
+ expressionType: 'SQL',
+ expression: 'case when a = 1 then 1 else 0 end',
+ },
+ },
+ queryObject,
+ ),
+ ).toEqual({
+ operation: 'pivot',
+ options: {
+ aggregates: {
+ 'count(*)': { operator: 'mean' },
+ 'count(*)__1 year ago': { operator: 'mean' },
+ 'count(*)__1 year later': { operator: 'mean' },
+ 'sum(val)': {
+ operator: 'mean',
+ },
+ 'sum(val)__1 year ago': {
+ operator: 'mean',
+ },
+ 'sum(val)__1 year later': {
+ operator: 'mean',
+ },
+ },
+ drop_missing_columns: false,
+ columns: ['foo', 'bar'],
+ index: ['my_case_expr'],
+ flatten_columns: false,
+ reset_index: false,
+ },
+ });
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index 1a2200db22..42e298be2c 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -23,6 +23,7 @@ import {
DataRecordValue,
DTTM_ALIAS,
GenericDataType,
+ getColumnLabel,
getNumberFormatter,
isEventAnnotationLayer,
isFormulaAnnotationLayer,
@@ -130,7 +131,8 @@ export default function transformProps(
const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
const rebasedData = rebaseForecastDatum(data, verboseMap);
- const xAxisCol = verboseMap[xAxisOrig] || xAxisOrig || DTTM_ALIAS;
+ const xAxisCol =
+ verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS);
const rawSeries = extractSeries(rebasedData, {
fillNeighborValue: stack && !forecastEnabled ? 0 : undefined,
xAxis: xAxisCol,
[superset] 07/10: fix(presto,trino): use correct literal dttm separator (#20123)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c5e2809c2cf62b5c120026b1d44b841857bddad7
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu May 19 20:42:31 2022 +0300
fix(presto,trino): use correct literal dttm separator (#20123)
* fix(presto,trino): use correct literal dttm separator
* remove redundant tests
(cherry picked from commit e2f11d3680a8f8a0fba7746b13551e49cdca7fd6)
---
superset/db_engine_specs/presto.py | 2 +-
superset/db_engine_specs/trino.py | 2 +-
tests/integration_tests/db_engine_specs/presto_tests.py | 13 -------------
tests/integration_tests/db_engine_specs/trino_tests.py | 13 -------------
tests/unit_tests/db_engine_specs/test_presto.py | 6 +++---
tests/unit_tests/db_engine_specs/test_trino.py | 6 +++---
6 files changed, 8 insertions(+), 34 deletions(-)
diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py
index a529980809..86b12b8538 100644
--- a/superset/db_engine_specs/presto.py
+++ b/superset/db_engine_specs/presto.py
@@ -758,7 +758,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
- return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds")}'"""
+ return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds", sep=" ")}'"""
return None
@classmethod
diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
index 29ffd024b4..4a5e9af01c 100644
--- a/superset/db_engine_specs/trino.py
+++ b/superset/db_engine_specs/trino.py
@@ -75,7 +75,7 @@ class TrinoEngineSpec(BaseEngineSpec):
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
- return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds")}'"""
+ return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds", sep=" ")}'"""
return None
@classmethod
diff --git a/tests/integration_tests/db_engine_specs/presto_tests.py b/tests/integration_tests/db_engine_specs/presto_tests.py
index acb9922d45..954f8d660a 100644
--- a/tests/integration_tests/db_engine_specs/presto_tests.py
+++ b/tests/integration_tests/db_engine_specs/presto_tests.py
@@ -512,19 +512,6 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
query_result = str(result.compile(compile_kwargs={"literal_binds": True}))
self.assertEqual("SELECT \nWHERE ds = '01-01-19' AND hour = 1", query_result)
- def test_convert_dttm(self):
- dttm = self.get_dttm()
-
- self.assertEqual(
- PrestoEngineSpec.convert_dttm("DATE", dttm),
- "DATE '2019-01-02'",
- )
-
- self.assertEqual(
- PrestoEngineSpec.convert_dttm("TIMESTAMP", dttm),
- "TIMESTAMP '2019-01-02T03:04:05.678900'",
- )
-
def test_query_cost_formatter(self):
raw_cost = [
{
diff --git a/tests/integration_tests/db_engine_specs/trino_tests.py b/tests/integration_tests/db_engine_specs/trino_tests.py
index 18f00b8988..b2235ae0c7 100644
--- a/tests/integration_tests/db_engine_specs/trino_tests.py
+++ b/tests/integration_tests/db_engine_specs/trino_tests.py
@@ -27,19 +27,6 @@ from tests.integration_tests.db_engine_specs.base_tests import TestDbEngineSpec
class TestTrinoDbEngineSpec(TestDbEngineSpec):
- def test_convert_dttm(self):
- dttm = self.get_dttm()
-
- self.assertEqual(
- TrinoEngineSpec.convert_dttm("DATE", dttm),
- "DATE '2019-01-02'",
- )
-
- self.assertEqual(
- TrinoEngineSpec.convert_dttm("TIMESTAMP", dttm),
- "TIMESTAMP '2019-01-02T03:04:05.678900'",
- )
-
def test_adjust_database_uri(self):
url = URL(drivername="trino", database="hive")
TrinoEngineSpec.adjust_database_uri(url, selected_schema="foobar")
diff --git a/tests/unit_tests/db_engine_specs/test_presto.py b/tests/unit_tests/db_engine_specs/test_presto.py
index 5bb327ff4e..512d03096b 100644
--- a/tests/unit_tests/db_engine_specs/test_presto.py
+++ b/tests/unit_tests/db_engine_specs/test_presto.py
@@ -30,17 +30,17 @@ from flask.ctx import AppContext
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "TIMESTAMP '2022-01-01T01:23:45.600000'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "TIMESTAMP '2022-01-01T01:23:45.600000'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
- "TIMESTAMP '2022-01-01T01:23:45.600000+00:00'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000+00:00'",
),
],
)
diff --git a/tests/unit_tests/db_engine_specs/test_trino.py b/tests/unit_tests/db_engine_specs/test_trino.py
index 1e800f3ad6..692fe875da 100644
--- a/tests/unit_tests/db_engine_specs/test_trino.py
+++ b/tests/unit_tests/db_engine_specs/test_trino.py
@@ -30,17 +30,17 @@ from flask.ctx import AppContext
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "TIMESTAMP '2022-01-01T01:23:45.600000'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "TIMESTAMP '2022-01-01T01:23:45.600000'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
- "TIMESTAMP '2022-01-01T01:23:45.600000+00:00'",
+ "TIMESTAMP '2022-01-01 01:23:45.600000+00:00'",
),
],
)
[superset] 02/10: fix(sqla): replace custom dttm type with literal_column (#19917)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9ca53b89052845c0d8f379b1c2286aea8b4e975e
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Tue May 3 07:19:36 2022 +0300
fix(sqla): replace custom dttm type with literal_column (#19917)
(cherry picked from commit 99f1f9ec610159db650081d9691835eea0f42b88)
---
superset/connectors/base/models.py | 31 ++++++++---
superset/connectors/druid/models.py | 2 +-
superset/connectors/sqla/models.py | 20 ++++---
superset/db_engine_specs/athena.py | 4 +-
superset/db_engine_specs/base.py | 9 +--
superset/db_engine_specs/presto.py | 6 +-
superset/db_engine_specs/trino.py | 4 +-
superset/models/sql_types/base.py | 65 ----------------------
.../db_engine_specs/presto_tests.py | 6 +-
.../db_engine_specs/trino_tests.py | 4 +-
tests/integration_tests/model_tests.py | 24 +-------
tests/unit_tests/db_engine_specs/test_athena.py | 9 +--
tests/unit_tests/db_engine_specs/test_presto.py | 8 +--
tests/unit_tests/db_engine_specs/test_trino.py | 8 +--
14 files changed, 63 insertions(+), 137 deletions(-)
diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index 9aacb0dc8c..7595141500 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -14,15 +14,18 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from __future__ import annotations
+
import json
from datetime import datetime
from enum import Enum
-from typing import Any, Dict, Hashable, List, Optional, Set, Type, Union
+from typing import Any, Dict, Hashable, List, Optional, Set, Type, TYPE_CHECKING, Union
from flask_appbuilder.security.sqla.models import User
from sqlalchemy import and_, Boolean, Column, Integer, String, Text
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.orm import foreign, Query, relationship, RelationshipProperty, Session
+from sqlalchemy.sql import literal_column
from superset import is_feature_enabled, security_manager
from superset.constants import EMPTY_STRING, NULL_STRING
@@ -33,6 +36,9 @@ from superset.superset_typing import FilterValue, FilterValues, QueryObjectDict
from superset.utils import core as utils
from superset.utils.core import GenericDataType
+if TYPE_CHECKING:
+ from superset.db_engine_specs.base import BaseEngineSpec
+
METRIC_FORM_DATA_PARAMS = [
"metric",
"metric_2",
@@ -387,25 +393,34 @@ class BaseDatasource(
return data
@staticmethod
- def filter_values_handler(
+ def filter_values_handler( # pylint: disable=too-many-arguments
values: Optional[FilterValues],
- target_column_type: utils.GenericDataType,
+ target_generic_type: GenericDataType,
+ target_native_type: Optional[str] = None,
is_list_target: bool = False,
+ db_engine_spec: Optional[Type[BaseEngineSpec]] = None,
+ db_extra: Optional[Dict[str, Any]] = None,
) -> Optional[FilterValues]:
if values is None:
return None
def handle_single_value(value: Optional[FilterValue]) -> Optional[FilterValue]:
- # backward compatibility with previous <select> components
if (
isinstance(value, (float, int))
- and target_column_type == utils.GenericDataType.TEMPORAL
+ and target_generic_type == utils.GenericDataType.TEMPORAL
+ and target_native_type is not None
+ and db_engine_spec is not None
):
- return datetime.utcfromtimestamp(value / 1000)
+ value = db_engine_spec.convert_dttm(
+ target_type=target_native_type,
+ dttm=datetime.utcfromtimestamp(value / 1000),
+ db_extra=db_extra,
+ )
+ value = literal_column(value)
if isinstance(value, str):
value = value.strip("\t\n")
- if target_column_type == utils.GenericDataType.NUMERIC:
+ if target_generic_type == utils.GenericDataType.NUMERIC:
# For backwards compatibility and edge cases
# where a column data type might have changed
return utils.cast_to_num(value)
@@ -413,7 +428,7 @@ class BaseDatasource(
return None
if value == EMPTY_STRING:
return ""
- if target_column_type == utils.GenericDataType.BOOLEAN:
+ if target_generic_type == utils.GenericDataType.BOOLEAN:
return utils.cast_to_boolean(value)
return value
diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py
index bc4d0ba381..4025c3f053 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -1533,7 +1533,7 @@ class DruidDatasource(Model, BaseDatasource):
eq = cls.filter_values_handler(
eq,
is_list_target=is_list_target,
- target_column_type=utils.GenericDataType.NUMERIC
+ target_generic_type=utils.GenericDataType.NUMERIC
if is_numeric_col
else utils.GenericDataType.STRING,
)
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 30e60a78c3..7289a7c3fb 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -1365,21 +1365,26 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
)
elif col_obj:
sqla_col = col_obj.get_sqla_col()
+ col_type = col_obj.type if col_obj else None
col_spec = db_engine_spec.get_column_spec(
- col_obj.type if col_obj else None
+ native_type=col_type,
+ db_extra=self.database.get_extra(),
)
is_list_target = op in (
utils.FilterOperator.IN.value,
utils.FilterOperator.NOT_IN.value,
)
if col_spec:
- target_type = col_spec.generic_type
+ target_generic_type = col_spec.generic_type
else:
- target_type = GenericDataType.STRING
+ target_generic_type = GenericDataType.STRING
eq = self.filter_values_handler(
values=val,
- target_column_type=target_type,
+ target_generic_type=target_generic_type,
+ target_native_type=col_type,
is_list_target=is_list_target,
+ db_engine_spec=db_engine_spec,
+ db_extra=self.database.get_extra(),
)
if is_list_target:
assert isinstance(eq, (tuple, list))
@@ -1387,11 +1392,12 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
raise QueryObjectValidationError(
_("Filter value list cannot be empty")
)
- if None in eq:
- eq = [x for x in eq if x is not None]
+ if len(eq) > len(
+ eq_without_none := [x for x in eq if x is not None]
+ ):
is_null_cond = sqla_col.is_(None)
if eq:
- cond = or_(is_null_cond, sqla_col.in_(eq))
+ cond = or_(is_null_cond, sqla_col.in_(eq_without_none))
else:
cond = is_null_cond
else:
diff --git a/superset/db_engine_specs/athena.py b/superset/db_engine_specs/athena.py
index 19bca87808..2394fe088d 100644
--- a/superset/db_engine_specs/athena.py
+++ b/superset/db_engine_specs/athena.py
@@ -67,10 +67,10 @@ class AthenaEngineSpec(BaseEngineSpec):
) -> Optional[str]:
tt = target_type.upper()
if tt == utils.TemporalType.DATE:
- return f"from_iso8601_date('{dttm.date().isoformat()}')"
+ return f"DATE '{dttm.date().isoformat()}'"
if tt == utils.TemporalType.TIMESTAMP:
datetime_formatted = dttm.isoformat(timespec="microseconds")
- return f"""from_iso8601_timestamp('{datetime_formatted}')"""
+ return f"""TIMESTAMP '{datetime_formatted}'"""
return None
@classmethod
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 123376bbb8..fb53222b43 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -60,7 +60,6 @@ from typing_extensions import TypedDict
from superset import security_manager, sql_parse
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.models.sql_lab import Query
-from superset.models.sql_types.base import literal_dttm_type_factory
from superset.sql_parse import ParsedQuery, Table
from superset.superset_typing import ResultSetColumnType
from superset.utils import core as utils
@@ -268,7 +267,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
(
re.compile(r"^date", re.IGNORECASE),
- types.DateTime(),
+ types.Date(),
GenericDataType.TEMPORAL,
),
(
@@ -1476,12 +1475,6 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
)
if col_types:
column_type, generic_type = col_types
- # wrap temporal types in custom type that supports literal binding
- # using datetimes
- if generic_type == GenericDataType.TEMPORAL:
- column_type = literal_dttm_type_factory(
- column_type, cls, native_type or "", db_extra=db_extra or {}
- )
is_dttm = generic_type == GenericDataType.TEMPORAL
return ColumnSpec(
sqla_type=column_type, generic_type=generic_type, is_dttm=is_dttm
diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py
index 77c3d4c2e3..a529980809 100644
--- a/superset/db_engine_specs/presto.py
+++ b/superset/db_engine_specs/presto.py
@@ -505,7 +505,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
),
(
re.compile(r"^date.*", re.IGNORECASE),
- types.DATETIME(),
+ types.DATE(),
GenericDataType.TEMPORAL,
),
(
@@ -753,12 +753,12 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
"""
tt = target_type.upper()
if tt == utils.TemporalType.DATE:
- return f"""from_iso8601_date('{dttm.date().isoformat()}')"""
+ return f"""DATE '{dttm.date().isoformat()}'"""
if tt in (
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
- return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')""" # pylint: disable=line-too-long,useless-suppression
+ return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds")}'"""
return None
@classmethod
diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
index 31e9a0aa7b..29ffd024b4 100644
--- a/superset/db_engine_specs/trino.py
+++ b/superset/db_engine_specs/trino.py
@@ -70,12 +70,12 @@ class TrinoEngineSpec(BaseEngineSpec):
"""
tt = target_type.upper()
if tt == utils.TemporalType.DATE:
- return f"from_iso8601_date('{dttm.date().isoformat()}')"
+ return f"DATE '{dttm.date().isoformat()}'"
if tt in (
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
- return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')""" # pylint: disable=line-too-long,useless-suppression
+ return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds")}'"""
return None
@classmethod
diff --git a/superset/models/sql_types/base.py b/superset/models/sql_types/base.py
deleted file mode 100644
index 597631a372..0000000000
--- a/superset/models/sql_types/base.py
+++ /dev/null
@@ -1,65 +0,0 @@
-# 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.
-from datetime import datetime
-from typing import Any, Callable, Dict, Type, TYPE_CHECKING
-
-from flask_babel import gettext as __
-from sqlalchemy import types
-from sqlalchemy.engine.interfaces import Dialect
-
-if TYPE_CHECKING:
- from superset.db_engine_specs.base import BaseEngineSpec
-
-
-def literal_dttm_type_factory(
- sqla_type: types.TypeEngine,
- db_engine_spec: Type["BaseEngineSpec"],
- col_type: str,
- db_extra: Dict[str, Any],
-) -> types.TypeEngine:
- """
- Create a custom SQLAlchemy type that supports datetime literal binds.
-
- :param sqla_type: Base type to extend
- :param db_engine_spec: Database engine spec which supports `convert_dttm` method
- :param col_type: native column type as defined in table metadata
- :param db_extra: The database extra object
- :return: SQLAlchemy type that supports using datetima as literal bind
- """
- # pylint: disable=too-few-public-methods
- class TemporalWrapperType(type(sqla_type)): # type: ignore
- # pylint: disable=unused-argument
- def literal_processor(self, dialect: Dialect) -> Callable[[Any], Any]:
- def process(value: Any) -> Any:
- if isinstance(value, datetime):
- ts_expression = db_engine_spec.convert_dttm(
- col_type, value, db_extra=db_extra
- )
- if ts_expression is None:
- raise NotImplementedError(
- __(
- "Temporal expression not supported for type: "
- "%(col_type)s",
- col_type=col_type,
- )
- )
- return ts_expression
- return super().process(value)
-
- return process
-
- return TemporalWrapperType()
diff --git a/tests/integration_tests/db_engine_specs/presto_tests.py b/tests/integration_tests/db_engine_specs/presto_tests.py
index 17c7c2d900..acb9922d45 100644
--- a/tests/integration_tests/db_engine_specs/presto_tests.py
+++ b/tests/integration_tests/db_engine_specs/presto_tests.py
@@ -517,12 +517,12 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
self.assertEqual(
PrestoEngineSpec.convert_dttm("DATE", dttm),
- "from_iso8601_date('2019-01-02')",
+ "DATE '2019-01-02'",
)
self.assertEqual(
PrestoEngineSpec.convert_dttm("TIMESTAMP", dttm),
- "from_iso8601_timestamp('2019-01-02T03:04:05.678900')",
+ "TIMESTAMP '2019-01-02T03:04:05.678900'",
)
def test_query_cost_formatter(self):
@@ -672,12 +672,10 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
column_spec = PrestoEngineSpec.get_column_spec("time")
assert isinstance(column_spec.sqla_type, types.Time)
- assert type(column_spec.sqla_type).__name__ == "TemporalWrapperType"
self.assertEqual(column_spec.generic_type, GenericDataType.TEMPORAL)
column_spec = PrestoEngineSpec.get_column_spec("timestamp")
assert isinstance(column_spec.sqla_type, types.TIMESTAMP)
- assert type(column_spec.sqla_type).__name__ == "TemporalWrapperType"
self.assertEqual(column_spec.generic_type, GenericDataType.TEMPORAL)
sqla_type = PrestoEngineSpec.get_sqla_column_type(None)
diff --git a/tests/integration_tests/db_engine_specs/trino_tests.py b/tests/integration_tests/db_engine_specs/trino_tests.py
index 973eb17159..18f00b8988 100644
--- a/tests/integration_tests/db_engine_specs/trino_tests.py
+++ b/tests/integration_tests/db_engine_specs/trino_tests.py
@@ -32,12 +32,12 @@ class TestTrinoDbEngineSpec(TestDbEngineSpec):
self.assertEqual(
TrinoEngineSpec.convert_dttm("DATE", dttm),
- "from_iso8601_date('2019-01-02')",
+ "DATE '2019-01-02'",
)
self.assertEqual(
TrinoEngineSpec.convert_dttm("TIMESTAMP", dttm),
- "from_iso8601_timestamp('2019-01-02T03:04:05.678900')",
+ "TIMESTAMP '2019-01-02T03:04:05.678900'",
)
def test_adjust_database_uri(self):
diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py
index 9f7962ae69..f57e03bffe 100644
--- a/tests/integration_tests/model_tests.py
+++ b/tests/integration_tests/model_tests.py
@@ -37,7 +37,6 @@ from superset.db_engine_specs.postgres import PostgresEngineSpec
from superset.common.db_query_status import QueryStatus
from superset.models.core import Database
from superset.models.slice import Slice
-from superset.models.sql_types.base import literal_dttm_type_factory
from superset.utils.database import get_example_database
from .base_tests import SupersetTestCase
@@ -376,30 +375,22 @@ class TestDatabaseModel(SupersetTestCase):
class TestSqlaTableModel(SupersetTestCase):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_get_timestamp_expression(self):
- col_type = (
- "VARCHAR"
- if get_example_database().backend == "presto"
- else "TemporalWrapperType"
- )
tbl = self.get_table(name="birth_names")
ds_col = tbl.get_column("ds")
sqla_literal = ds_col.get_timestamp_expression(None)
- self.assertEqual(str(sqla_literal.compile()), "ds")
- assert type(sqla_literal.type).__name__ == col_type
+ assert str(sqla_literal.compile()) == "ds"
sqla_literal = ds_col.get_timestamp_expression("P1D")
- assert type(sqla_literal.type).__name__ == col_type
compiled = "{}".format(sqla_literal.compile())
if tbl.database.backend == "mysql":
- self.assertEqual(compiled, "DATE(ds)")
+ assert compiled == "DATE(ds)"
prev_ds_expr = ds_col.expression
ds_col.expression = "DATE_ADD(ds, 1)"
sqla_literal = ds_col.get_timestamp_expression("P1D")
- assert type(sqla_literal.type).__name__ == col_type
compiled = "{}".format(sqla_literal.compile())
if tbl.database.backend == "mysql":
- self.assertEqual(compiled, "DATE(DATE_ADD(ds, 1))")
+ assert compiled == "DATE(DATE_ADD(ds, 1))"
ds_col.expression = prev_ds_expr
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
@@ -615,12 +606,3 @@ class TestSqlaTableModel(SupersetTestCase):
# clean up and auto commit
metadata_db.session.delete(slc)
-
-
-def test_literal_dttm_type_factory():
- orig_type = DateTime()
- new_type = literal_dttm_type_factory(
- orig_type, PostgresEngineSpec, "TIMESTAMP", db_extra={}
- )
- assert type(new_type).__name__ == "TemporalWrapperType"
- assert str(new_type) == str(orig_type)
diff --git a/tests/unit_tests/db_engine_specs/test_athena.py b/tests/unit_tests/db_engine_specs/test_athena.py
index 32a401f595..a35d8eddfb 100644
--- a/tests/unit_tests/db_engine_specs/test_athena.py
+++ b/tests/unit_tests/db_engine_specs/test_athena.py
@@ -35,13 +35,11 @@ def test_convert_dttm(app_context: AppContext, dttm: datetime) -> None:
from superset.db_engine_specs.athena import AthenaEngineSpec
- assert (
- AthenaEngineSpec.convert_dttm("DATE", dttm) == "from_iso8601_date('2019-01-02')"
- )
+ assert AthenaEngineSpec.convert_dttm("DATE", dttm) == "DATE '2019-01-02'"
assert (
AthenaEngineSpec.convert_dttm("TIMESTAMP", dttm)
- == "from_iso8601_timestamp('2019-01-02T03:04:05.678900')"
+ == "TIMESTAMP '2019-01-02T03:04:05.678900'"
)
@@ -80,8 +78,7 @@ def test_get_text_clause_with_colon(app_context: AppContext) -> None:
from superset.db_engine_specs.athena import AthenaEngineSpec
query = (
- "SELECT foo FROM tbl WHERE "
- "abc >= from_iso8601_timestamp('2021-11-26T00\:00\:00.000000')"
+ "SELECT foo FROM tbl WHERE " "abc >= TIMESTAMP '2021-11-26T00\:00\:00.000000'"
)
text_clause = AthenaEngineSpec.get_text_clause(query)
assert text_clause.text == query
diff --git a/tests/unit_tests/db_engine_specs/test_presto.py b/tests/unit_tests/db_engine_specs/test_presto.py
index eea6b6ec3c..5bb327ff4e 100644
--- a/tests/unit_tests/db_engine_specs/test_presto.py
+++ b/tests/unit_tests/db_engine_specs/test_presto.py
@@ -26,21 +26,21 @@ from flask.ctx import AppContext
"target_type,dttm,result",
[
("VARCHAR", datetime(2022, 1, 1), None),
- ("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
+ ("DATE", datetime(2022, 1, 1), "DATE '2022-01-01'"),
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000+00:00'",
),
],
)
diff --git a/tests/unit_tests/db_engine_specs/test_trino.py b/tests/unit_tests/db_engine_specs/test_trino.py
index 9962a0f66d..1e800f3ad6 100644
--- a/tests/unit_tests/db_engine_specs/test_trino.py
+++ b/tests/unit_tests/db_engine_specs/test_trino.py
@@ -26,21 +26,21 @@ from flask.ctx import AppContext
"target_type,dttm,result",
[
("VARCHAR", datetime(2022, 1, 1), None),
- ("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
+ ("DATE", datetime(2022, 1, 1), "DATE '2022-01-01'"),
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000'",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
- "from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
+ "TIMESTAMP '2022-01-01T01:23:45.600000+00:00'",
),
],
)
[superset] 10/10: fix(temporary-cache): when user is anonymous (#20181)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8089afce1e74e4f697043a7fa105aa520d3ac3e7
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu May 26 14:45:20 2022 +0300
fix(temporary-cache): when user is anonymous (#20181)
* fix(temporary-cache): fail on anonymous user
* make exceptions generic
* fix test
* remove redundant bool return
* fix unit tests
(cherry picked from commit 64c4226817b04ff598be29b52d8e2c4a679ef70a)
---
.../dashboards/filter_state/commands/create.py | 17 ++++----
.../dashboards/filter_state/commands/delete.py | 22 +++++-----
superset/dashboards/filter_state/commands/get.py | 4 +-
.../dashboards/filter_state/commands/update.py | 45 +++++++++----------
.../filter_state/commands/utils.py} | 23 +++++++---
superset/explore/form_data/api.py | 45 +++++--------------
superset/explore/form_data/commands/create.py | 6 +--
superset/explore/form_data/commands/delete.py | 5 ++-
superset/explore/form_data/commands/get.py | 2 +-
superset/explore/form_data/commands/state.py | 2 +-
superset/explore/form_data/commands/update.py | 10 ++---
superset/explore/form_data/commands/utils.py | 42 ++++++++++++++++++
superset/explore/utils.py | 8 ++--
superset/key_value/utils.py | 7 ++-
superset/temporary_cache/api.py | 51 +++++-----------------
superset/temporary_cache/commands/entry.py | 4 +-
superset/temporary_cache/commands/exceptions.py | 4 ++
tests/unit_tests/explore/utils_test.py | 8 ++--
18 files changed, 156 insertions(+), 149 deletions(-)
diff --git a/superset/dashboards/filter_state/commands/create.py b/superset/dashboards/filter_state/commands/create.py
index 137623027a..18dff8928f 100644
--- a/superset/dashboards/filter_state/commands/create.py
+++ b/superset/dashboards/filter_state/commands/create.py
@@ -14,11 +14,13 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from typing import cast
+
from flask import session
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
from superset.temporary_cache.commands.create import CreateTemporaryCacheCommand
from superset.temporary_cache.commands.entry import Entry
from superset.temporary_cache.commands.parameters import CommandParameters
@@ -34,10 +36,9 @@ class CreateFilterStateCommand(CreateTemporaryCacheCommand):
key = cache_manager.filter_state_cache.get(contextual_key)
if not key or not tab_id:
key = random_key()
- value = cmd_params.value
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard and value:
- entry: Entry = {"owner": actor.get_user_id(), "value": value}
- cache_manager.filter_state_cache.set(cache_key(resource_id, key), entry)
- cache_manager.filter_state_cache.set(contextual_key, key)
+ value = cast(str, cmd_params.value) # schema ensures that value is not optional
+ check_access(resource_id)
+ entry: Entry = {"owner": get_owner(actor), "value": value}
+ cache_manager.filter_state_cache.set(cache_key(resource_id, key), entry)
+ cache_manager.filter_state_cache.set(contextual_key, key)
return key
diff --git a/superset/dashboards/filter_state/commands/delete.py b/superset/dashboards/filter_state/commands/delete.py
index 155c63f108..3ddc08fc51 100644
--- a/superset/dashboards/filter_state/commands/delete.py
+++ b/superset/dashboards/filter_state/commands/delete.py
@@ -16,8 +16,9 @@
# under the License.
from flask import session
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
from superset.extensions import cache_manager
+from superset.key_value.utils import get_owner
from superset.temporary_cache.commands.delete import DeleteTemporaryCacheCommand
from superset.temporary_cache.commands.entry import Entry
from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
@@ -30,14 +31,13 @@ class DeleteFilterStateCommand(DeleteTemporaryCacheCommand):
resource_id = cmd_params.resource_id
actor = cmd_params.actor
key = cache_key(resource_id, cmd_params.key)
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
- entry: Entry = cache_manager.filter_state_cache.get(key)
- if entry:
- if entry["owner"] != actor.get_user_id():
- raise TemporaryCacheAccessDeniedError()
- tab_id = cmd_params.tab_id
- contextual_key = cache_key(session.get("_id"), tab_id, resource_id)
- cache_manager.filter_state_cache.delete(contextual_key)
- return cache_manager.filter_state_cache.delete(key)
+ check_access(resource_id)
+ entry: Entry = cache_manager.filter_state_cache.get(key)
+ if entry:
+ if entry["owner"] != get_owner(actor):
+ raise TemporaryCacheAccessDeniedError()
+ tab_id = cmd_params.tab_id
+ contextual_key = cache_key(session.get("_id"), tab_id, resource_id)
+ cache_manager.filter_state_cache.delete(contextual_key)
+ return cache_manager.filter_state_cache.delete(key)
return False
diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py
index 9cdd5bcddc..ca7ffa9879 100644
--- a/superset/dashboards/filter_state/commands/get.py
+++ b/superset/dashboards/filter_state/commands/get.py
@@ -18,7 +18,7 @@ from typing import Optional
from flask import current_app as app
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
from superset.extensions import cache_manager
from superset.temporary_cache.commands.get import GetTemporaryCacheCommand
from superset.temporary_cache.commands.parameters import CommandParameters
@@ -34,7 +34,7 @@ class GetFilterStateCommand(GetTemporaryCacheCommand):
def get(self, cmd_params: CommandParameters) -> Optional[str]:
resource_id = cmd_params.resource_id
key = cache_key(resource_id, cmd_params.key)
- DashboardDAO.get_by_id_or_slug(str(resource_id))
+ check_access(resource_id)
entry = cache_manager.filter_state_cache.get(key) or {}
if entry and self._refresh_timeout:
cache_manager.filter_state_cache.set(key, entry)
diff --git a/superset/dashboards/filter_state/commands/update.py b/superset/dashboards/filter_state/commands/update.py
index d27277f9af..7f150aae6b 100644
--- a/superset/dashboards/filter_state/commands/update.py
+++ b/superset/dashboards/filter_state/commands/update.py
@@ -14,13 +14,13 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
+from typing import cast, Optional
from flask import session
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
from superset.temporary_cache.commands.entry import Entry
from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
from superset.temporary_cache.commands.parameters import CommandParameters
@@ -33,28 +33,23 @@ class UpdateFilterStateCommand(UpdateTemporaryCacheCommand):
resource_id = cmd_params.resource_id
actor = cmd_params.actor
key = cmd_params.key
- value = cmd_params.value
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard and value:
- entry: Entry = cache_manager.filter_state_cache.get(
- cache_key(resource_id, key)
- )
- if entry:
- user_id = actor.get_user_id()
- if entry["owner"] != user_id:
- raise TemporaryCacheAccessDeniedError()
+ value = cast(str, cmd_params.value) # schema ensures that value is not optional
+ check_access(resource_id)
+ entry: Entry = cache_manager.filter_state_cache.get(cache_key(resource_id, key))
+ owner = get_owner(actor)
+ if entry:
+ if entry["owner"] != owner:
+ raise TemporaryCacheAccessDeniedError()
- # Generate a new key if tab_id changes or equals 0
- contextual_key = cache_key(
- session.get("_id"), cmd_params.tab_id, resource_id
- )
- key = cache_manager.filter_state_cache.get(contextual_key)
- if not key or not cmd_params.tab_id:
- key = random_key()
- cache_manager.filter_state_cache.set(contextual_key, key)
+ # Generate a new key if tab_id changes or equals 0
+ contextual_key = cache_key(
+ session.get("_id"), cmd_params.tab_id, resource_id
+ )
+ key = cache_manager.filter_state_cache.get(contextual_key)
+ if not key or not cmd_params.tab_id:
+ key = random_key()
+ cache_manager.filter_state_cache.set(contextual_key, key)
- new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
- cache_manager.filter_state_cache.set(
- cache_key(resource_id, key), new_entry
- )
+ new_entry: Entry = {"owner": owner, "value": value}
+ cache_manager.filter_state_cache.set(cache_key(resource_id, key), new_entry)
return key
diff --git a/superset/explore/form_data/commands/state.py b/superset/dashboards/filter_state/commands/utils.py
similarity index 56%
copy from superset/explore/form_data/commands/state.py
copy to superset/dashboards/filter_state/commands/utils.py
index 2aba14a8cb..35f940f434 100644
--- a/superset/explore/form_data/commands/state.py
+++ b/superset/dashboards/filter_state/commands/utils.py
@@ -14,13 +14,22 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-from typing_extensions import TypedDict
+from superset.dashboards.commands.exceptions import (
+ DashboardAccessDeniedError,
+ DashboardNotFoundError,
+)
+from superset.dashboards.dao import DashboardDAO
+from superset.temporary_cache.commands.exceptions import (
+ TemporaryCacheAccessDeniedError,
+ TemporaryCacheResourceNotFoundError,
+)
-class TemporaryExploreState(TypedDict):
- owner: int
- dataset_id: int
- chart_id: Optional[int]
- form_data: str
+def check_access(resource_id: int) -> None:
+ try:
+ DashboardDAO.get_by_id_or_slug(str(resource_id))
+ except DashboardNotFoundError as ex:
+ raise TemporaryCacheResourceNotFoundError from ex
+ except DashboardAccessDeniedError as ex:
+ raise TemporaryCacheAccessDeniedError from ex
diff --git a/superset/explore/form_data/api.py b/superset/explore/form_data/api.py
index dc6ee7ea94..ea2b38658b 100644
--- a/superset/explore/form_data/api.py
+++ b/superset/explore/form_data/api.py
@@ -21,15 +21,7 @@ from flask import g, request, Response
from flask_appbuilder.api import BaseApi, expose, protect, safe
from marshmallow import ValidationError
-from superset.charts.commands.exceptions import (
- ChartAccessDeniedError,
- ChartNotFoundError,
-)
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
-from superset.datasets.commands.exceptions import (
- DatasetAccessDeniedError,
- DatasetNotFoundError,
-)
from superset.explore.form_data.commands.create import CreateFormDataCommand
from superset.explore.form_data.commands.delete import DeleteFormDataCommand
from superset.explore.form_data.commands.get import GetFormDataCommand
@@ -37,7 +29,10 @@ from superset.explore.form_data.commands.parameters import CommandParameters
from superset.explore.form_data.commands.update import UpdateFormDataCommand
from superset.explore.form_data.schemas import FormDataPostSchema, FormDataPutSchema
from superset.extensions import event_logger
-from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
+from superset.temporary_cache.commands.exceptions import (
+ TemporaryCacheAccessDeniedError,
+ TemporaryCacheResourceNotFoundError,
+)
from superset.views.base_api import requires_json
logger = logging.getLogger(__name__)
@@ -118,13 +113,9 @@ class ExploreFormDataRestApi(BaseApi, ABC):
return self.response(201, key=key)
except ValidationError as ex:
return self.response(400, message=ex.messages)
- except (
- ChartAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
@expose("/form_data/<string:key>", methods=["PUT"])
@@ -195,13 +186,9 @@ class ExploreFormDataRestApi(BaseApi, ABC):
return self.response(200, key=result)
except ValidationError as ex:
return self.response(400, message=ex.messages)
- except (
- ChartAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
@expose("/form_data/<string:key>", methods=["GET"])
@@ -250,13 +237,9 @@ class ExploreFormDataRestApi(BaseApi, ABC):
if not form_data:
return self.response_404()
return self.response(200, form_data=form_data)
- except (
- ChartAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
@expose("/form_data/<string:key>", methods=["DELETE"])
@@ -306,11 +289,7 @@ class ExploreFormDataRestApi(BaseApi, ABC):
if not result:
return self.response_404()
return self.response(200, message="Deleted successfully")
- except (
- ChartAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
diff --git a/superset/explore/form_data/commands/create.py b/superset/explore/form_data/commands/create.py
index 7b1f866c50..84ad0f0078 100644
--- a/superset/explore/form_data/commands/create.py
+++ b/superset/explore/form_data/commands/create.py
@@ -22,9 +22,9 @@ from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.explore.form_data.commands.parameters import CommandParameters
from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
from superset.temporary_cache.commands.exceptions import TemporaryCacheCreateFailedError
from superset.temporary_cache.utils import cache_key
from superset.utils.schema import validate_json
@@ -51,7 +51,7 @@ class CreateFormDataCommand(BaseCommand):
key = random_key()
if form_data:
state: TemporaryExploreState = {
- "owner": actor.get_user_id(),
+ "owner": get_owner(actor),
"dataset_id": dataset_id,
"chart_id": chart_id,
"form_data": form_data,
diff --git a/superset/explore/form_data/commands/delete.py b/superset/explore/form_data/commands/delete.py
index ec537313d2..69d5a79b8f 100644
--- a/superset/explore/form_data/commands/delete.py
+++ b/superset/explore/form_data/commands/delete.py
@@ -23,8 +23,9 @@ from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.explore.form_data.commands.parameters import CommandParameters
from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
from superset.extensions import cache_manager
+from superset.key_value.utils import get_owner
from superset.temporary_cache.commands.exceptions import (
TemporaryCacheAccessDeniedError,
TemporaryCacheDeleteFailedError,
@@ -49,7 +50,7 @@ class DeleteFormDataCommand(BaseCommand, ABC):
dataset_id = state["dataset_id"]
chart_id = state["chart_id"]
check_access(dataset_id, chart_id, actor)
- if state["owner"] != actor.get_user_id():
+ if state["owner"] != get_owner(actor):
raise TemporaryCacheAccessDeniedError()
tab_id = self._cmd_params.tab_id
contextual_key = cache_key(
diff --git a/superset/explore/form_data/commands/get.py b/superset/explore/form_data/commands/get.py
index 5b58200821..809672f32f 100644
--- a/superset/explore/form_data/commands/get.py
+++ b/superset/explore/form_data/commands/get.py
@@ -24,7 +24,7 @@ from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.explore.form_data.commands.parameters import CommandParameters
from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
from superset.extensions import cache_manager
from superset.temporary_cache.commands.exceptions import TemporaryCacheGetFailedError
diff --git a/superset/explore/form_data/commands/state.py b/superset/explore/form_data/commands/state.py
index 2aba14a8cb..c8061e81f5 100644
--- a/superset/explore/form_data/commands/state.py
+++ b/superset/explore/form_data/commands/state.py
@@ -20,7 +20,7 @@ from typing_extensions import TypedDict
class TemporaryExploreState(TypedDict):
- owner: int
+ owner: Optional[int]
dataset_id: int
chart_id: Optional[int]
form_data: str
diff --git a/superset/explore/form_data/commands/update.py b/superset/explore/form_data/commands/update.py
index 596c5f6e27..76dfee1dad 100644
--- a/superset/explore/form_data/commands/update.py
+++ b/superset/explore/form_data/commands/update.py
@@ -24,9 +24,9 @@ from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.explore.form_data.commands.parameters import CommandParameters
from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
from superset.temporary_cache.commands.exceptions import (
TemporaryCacheAccessDeniedError,
TemporaryCacheUpdateFailedError,
@@ -56,9 +56,9 @@ class UpdateFormDataCommand(BaseCommand, ABC):
state: TemporaryExploreState = cache_manager.explore_form_data_cache.get(
key
)
+ owner = get_owner(actor)
if state and form_data:
- user_id = actor.get_user_id()
- if state["owner"] != user_id:
+ if state["owner"] != owner:
raise TemporaryCacheAccessDeniedError()
# Generate a new key if tab_id changes or equals 0
@@ -72,7 +72,7 @@ class UpdateFormDataCommand(BaseCommand, ABC):
cache_manager.explore_form_data_cache.set(contextual_key, key)
new_state: TemporaryExploreState = {
- "owner": actor.get_user_id(),
+ "owner": owner,
"dataset_id": dataset_id,
"chart_id": chart_id,
"form_data": form_data,
diff --git a/superset/explore/form_data/commands/utils.py b/superset/explore/form_data/commands/utils.py
new file mode 100644
index 0000000000..5d09657fbe
--- /dev/null
+++ b/superset/explore/form_data/commands/utils.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.
+from typing import Optional
+
+from flask_appbuilder.security.sqla.models import User
+
+from superset.charts.commands.exceptions import (
+ ChartAccessDeniedError,
+ ChartNotFoundError,
+)
+from superset.datasets.commands.exceptions import (
+ DatasetAccessDeniedError,
+ DatasetNotFoundError,
+)
+from superset.explore.utils import check_access as explore_check_access
+from superset.temporary_cache.commands.exceptions import (
+ TemporaryCacheAccessDeniedError,
+ TemporaryCacheResourceNotFoundError,
+)
+
+
+def check_access(dataset_id: int, chart_id: Optional[int], actor: User) -> None:
+ try:
+ explore_check_access(dataset_id, chart_id, actor)
+ except (ChartNotFoundError, DatasetNotFoundError) as ex:
+ raise TemporaryCacheResourceNotFoundError from ex
+ except (ChartAccessDeniedError, DatasetAccessDeniedError) as ex:
+ raise TemporaryCacheAccessDeniedError from ex
diff --git a/superset/explore/utils.py b/superset/explore/utils.py
index 3eeb1bab99..7ab29de2f7 100644
--- a/superset/explore/utils.py
+++ b/superset/explore/utils.py
@@ -44,12 +44,10 @@ def check_dataset_access(dataset_id: int) -> Optional[bool]:
raise DatasetNotFoundError()
-def check_access(
- dataset_id: int, chart_id: Optional[int], actor: User
-) -> Optional[bool]:
+def check_access(dataset_id: int, chart_id: Optional[int], actor: User) -> None:
check_dataset_access(dataset_id)
if not chart_id:
- return True
+ return
chart = ChartDAO.find_by_id(chart_id)
if chart:
can_access_chart = (
@@ -58,6 +56,6 @@ def check_access(
or security_manager.can_access("can_read", "Chart")
)
if can_access_chart:
- return True
+ return
raise ChartAccessDeniedError()
raise ChartNotFoundError()
diff --git a/superset/key_value/utils.py b/superset/key_value/utils.py
index b2e8e729b0..db27e505fb 100644
--- a/superset/key_value/utils.py
+++ b/superset/key_value/utils.py
@@ -18,10 +18,11 @@ from __future__ import annotations
from hashlib import md5
from secrets import token_urlsafe
-from typing import Union
+from typing import Optional, Union
from uuid import UUID
import hashids
+from flask_appbuilder.security.sqla.models import User
from flask_babel import gettext as _
from superset.key_value.exceptions import KeyValueParseKeyError
@@ -63,3 +64,7 @@ def get_uuid_namespace(seed: str) -> UUID:
md5_obj = md5()
md5_obj.update(seed.encode("utf-8"))
return UUID(md5_obj.hexdigest())
+
+
+def get_owner(user: User) -> Optional[int]:
+ return user.get_user_id() if not user.is_anonymous else None
diff --git a/superset/temporary_cache/api.py b/superset/temporary_cache/api.py
index e91a288669..bdbdda302e 100644
--- a/superset/temporary_cache/api.py
+++ b/superset/temporary_cache/api.py
@@ -24,20 +24,11 @@ from flask import g, request, Response
from flask_appbuilder.api import BaseApi
from marshmallow import ValidationError
-from superset.charts.commands.exceptions import (
- ChartAccessDeniedError,
- ChartNotFoundError,
-)
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
-from superset.dashboards.commands.exceptions import (
- DashboardAccessDeniedError,
- DashboardNotFoundError,
-)
-from superset.datasets.commands.exceptions import (
- DatasetAccessDeniedError,
- DatasetNotFoundError,
+from superset.temporary_cache.commands.exceptions import (
+ TemporaryCacheAccessDeniedError,
+ TemporaryCacheResourceNotFoundError,
)
-from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
from superset.temporary_cache.commands.parameters import CommandParameters
from superset.temporary_cache.schemas import (
TemporaryCachePostSchema,
@@ -86,14 +77,9 @@ class TemporaryCacheRestApi(BaseApi, ABC):
return self.response(201, key=key)
except ValidationError as ex:
return self.response(400, message=ex.messages)
- except (
- ChartAccessDeniedError,
- DashboardAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
@requires_json
@@ -112,14 +98,9 @@ class TemporaryCacheRestApi(BaseApi, ABC):
return self.response(200, key=key)
except ValidationError as ex:
return self.response(400, message=ex.messages)
- except (
- ChartAccessDeniedError,
- DashboardAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
def get(self, pk: int, key: str) -> Response:
@@ -129,14 +110,9 @@ class TemporaryCacheRestApi(BaseApi, ABC):
if not value:
return self.response_404()
return self.response(200, value=value)
- except (
- ChartAccessDeniedError,
- DashboardAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
def delete(self, pk: int, key: str) -> Response:
@@ -146,14 +122,9 @@ class TemporaryCacheRestApi(BaseApi, ABC):
if not result:
return self.response_404()
return self.response(200, message="Deleted successfully")
- except (
- ChartAccessDeniedError,
- DashboardAccessDeniedError,
- DatasetAccessDeniedError,
- TemporaryCacheAccessDeniedError,
- ) as ex:
+ except TemporaryCacheAccessDeniedError as ex:
return self.response(403, message=str(ex))
- except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ except TemporaryCacheResourceNotFoundError as ex:
return self.response(404, message=str(ex))
@abstractmethod
diff --git a/superset/temporary_cache/commands/entry.py b/superset/temporary_cache/commands/entry.py
index 0e9ad0a735..90aa8e1beb 100644
--- a/superset/temporary_cache/commands/entry.py
+++ b/superset/temporary_cache/commands/entry.py
@@ -14,9 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from typing import Optional
+
from typing_extensions import TypedDict
class Entry(TypedDict):
- owner: int
+ owner: Optional[int]
value: str
diff --git a/superset/temporary_cache/commands/exceptions.py b/superset/temporary_cache/commands/exceptions.py
index 0f8c44cb18..8652a732f7 100644
--- a/superset/temporary_cache/commands/exceptions.py
+++ b/superset/temporary_cache/commands/exceptions.py
@@ -43,3 +43,7 @@ class TemporaryCacheUpdateFailedError(UpdateFailedError):
class TemporaryCacheAccessDeniedError(ForbiddenError):
message = _("You don't have permission to modify the value.")
+
+
+class TemporaryCacheResourceNotFoundError(ForbiddenError):
+ message = _("Resource was not found.")
diff --git a/tests/unit_tests/explore/utils_test.py b/tests/unit_tests/explore/utils_test.py
index 3d12f5e911..11e2906ed8 100644
--- a/tests/unit_tests/explore/utils_test.py
+++ b/tests/unit_tests/explore/utils_test.py
@@ -75,7 +75,7 @@ def test_unsaved_chart_authorized_dataset(
mocker.patch(dataset_find_by_id, return_value=SqlaTable())
mocker.patch(can_access_datasource, return_value=True)
- assert check_access(dataset_id=1, chart_id=0, actor=User()) == True
+ check_access(dataset_id=1, chart_id=0, actor=User())
def test_saved_chart_unknown_chart_id(
@@ -112,7 +112,7 @@ def test_saved_chart_is_admin(mocker: MockFixture, app_context: AppContext) -> N
mocker.patch(can_access_datasource, return_value=True)
mocker.patch(is_user_admin, return_value=True)
mocker.patch(chart_find_by_id, return_value=Slice())
- assert check_access(dataset_id=1, chart_id=1, actor=User()) is True
+ check_access(dataset_id=1, chart_id=1, actor=User())
def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> None:
@@ -125,7 +125,7 @@ def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> N
mocker.patch(is_user_admin, return_value=False)
mocker.patch(is_owner, return_value=True)
mocker.patch(chart_find_by_id, return_value=Slice())
- assert check_access(dataset_id=1, chart_id=1, actor=User()) == True
+ check_access(dataset_id=1, chart_id=1, actor=User())
def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) -> None:
@@ -139,7 +139,7 @@ def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) ->
mocker.patch(is_owner, return_value=False)
mocker.patch(can_access, return_value=True)
mocker.patch(chart_find_by_id, return_value=Slice())
- assert check_access(dataset_id=1, chart_id=1, actor=User()) == True
+ check_access(dataset_id=1, chart_id=1, actor=User())
def test_saved_chart_no_access(mocker: MockFixture, app_context: AppContext) -> None:
[superset] 01/10: fix: Alpha should not be able to edit datasets that they don't own (#19854)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2bd89d1705347da5446902a3f65eb8d0a6353503
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Fri Apr 29 14:21:05 2022 -0700
fix: Alpha should not be able to edit datasets that they don't own (#19854)
* fix api for checking owners
* fix styles for disabling
* fix styles for disabling
* fix lint
* fix lint
* add owners key
* plzz
* remove
* update test
* add tooltip
* add type
* fix test
* fix user reference
* lit
* fix test
* work
(cherry picked from commit 8b15b68979bf033979fe7014ef2730095ae85120)
---
.../src/shared-controls/index.tsx | 6 ++++
.../DatasourcePanel/DatasourcePanel.test.tsx | 17 +++++++++
.../explore/components/DatasourcePanel/index.tsx | 2 ++
.../DatasourceControl/DatasourceControl.test.jsx | 13 +++++++
.../DatasourceControl/DatasourceControl.test.tsx | 11 ++++++
.../controls/DatasourceControl/index.jsx | 25 ++++++++++++--
.../views/CRUD/data/dataset/DatasetList.test.jsx | 1 +
.../src/views/CRUD/data/dataset/DatasetList.tsx | 40 +++++++++++++++++++---
superset/views/datasource/views.py | 34 ++++++------------
tests/integration_tests/datasource_tests.py | 8 +++++
10 files changed, 127 insertions(+), 30 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
index 10593b3c71..42c728e1d3 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
@@ -91,6 +91,11 @@ export const PRIMARY_COLOR = { r: 0, g: 122, b: 135, a: 1 };
const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];
const SERIES_LIMITS = [5, 10, 25, 50, 100, 500];
+const appContainer = document.getElementById('app');
+const { user } = JSON.parse(
+ appContainer?.getAttribute('data-bootstrap') || '{}',
+);
+
type Control = {
savedMetrics?: Metric[] | null;
default?: unknown;
@@ -167,6 +172,7 @@ const datasourceControl: SharedControlConfig<'DatasourceControl'> = {
mapStateToProps: ({ datasource, form_data }) => ({
datasource,
form_data,
+ user,
}),
};
diff --git a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx
index 0f11f48dd8..99c596b80e 100644
--- a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx
+++ b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx
@@ -47,7 +47,22 @@ const datasource = {
main_dttm_col: 'None',
datasource_name: 'table1',
description: 'desc',
+ owners: [{ username: 'admin', userId: 1 }],
};
+
+const mockUser = {
+ createdOn: '2021-04-27T18:12:38.952304',
+ email: 'admin',
+ firstName: 'admin',
+ isActive: true,
+ lastName: 'admin',
+ permissions: {},
+ roles: { Admin: Array(173) },
+ userId: 1,
+ username: 'admin',
+ isAnonymous: false,
+};
+
const props: DatasourcePanelProps = {
datasource,
controls: {
@@ -57,6 +72,7 @@ const props: DatasourcePanelProps = {
type: DatasourceControl,
label: 'hello',
datasource,
+ user: mockUser,
},
},
actions: {
@@ -154,6 +170,7 @@ test('should render a warning', async () => {
datasource: {
...props.controls.datasource,
datasource: deprecatedDatasource,
+ user: mockUser,
},
},
}),
diff --git a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx
index 04378cb998..9b033d2019 100644
--- a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx
+++ b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx
@@ -31,12 +31,14 @@ import { FAST_DEBOUNCE } from 'src/constants';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { ExploreActions } from 'src/explore/actions/exploreActions';
import Control from 'src/explore/components/Control';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import DatasourcePanelDragOption from './DatasourcePanelDragOption';
import { DndItemType } from '../DndItemType';
import { StyledColumnOption, StyledMetricOption } from '../optionRenderers';
interface DatasourceControl extends ControlConfig {
datasource?: DatasourceMeta;
+ user: UserWithPermissionsAndRoles;
}
export interface Props {
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.jsx
index 73c2777663..41a40c0bc4 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.jsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.jsx
@@ -41,6 +41,7 @@ const defaultProps = {
id: 1,
columns: [],
metrics: [],
+ owners: [{ username: 'admin', userId: 1 }],
database: {
backend: 'mysql',
name: 'main',
@@ -51,6 +52,17 @@ const defaultProps = {
setDatasource: sinon.spy(),
},
onChange: sinon.spy(),
+ user: {
+ createdOn: '2021-04-27T18:12:38.952304',
+ email: 'admin',
+ firstName: 'admin',
+ isActive: true,
+ lastName: 'admin',
+ permissions: {},
+ roles: { Admin: Array(173) },
+ userId: 1,
+ username: 'admin',
+ },
};
describe('DatasourceControl', () => {
@@ -107,6 +119,7 @@ describe('DatasourceControl', () => {
id: 1,
columns: [],
metrics: [],
+ owners: [{ username: 'admin', userId: 1 }],
database: {
backend: 'druid',
name: 'main',
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
index f4f66ef43e..4811743489 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -48,6 +48,17 @@ const createProps = () => ({
name: 'datasource',
actions: {},
isEditable: true,
+ user: {
+ createdOn: '2021-04-27T18:12:38.952304',
+ email: 'admin',
+ firstName: 'admin',
+ isActive: true,
+ lastName: 'admin',
+ permissions: {},
+ roles: { Admin: Array(173) },
+ userId: 1,
+ username: 'admin',
+ },
onChange: jest.fn(),
onDatasourceSave: jest.fn(),
});
diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
index f7cda8b752..661f934041 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
@@ -35,6 +35,7 @@ import Button from 'src/components/Button';
import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { URL_PARAMS } from 'src/constants';
+import { isUserAdmin } from 'src/dashboard/util/findPermission';
const propTypes = {
actions: PropTypes.object.isRequired,
@@ -195,12 +196,32 @@ class DatasourceControl extends React.PureComponent {
}
const isSqlSupported = datasource.type === 'table';
+ const { user } = this.props;
+ const allowEdit =
+ datasource.owners.map(o => o.id).includes(user.userId) ||
+ isUserAdmin(user);
+
+ const editText = t('Edit dataset');
const datasourceMenu = (
<Menu onClick={this.handleMenuItemClick}>
{this.props.isEditable && (
- <Menu.Item key={EDIT_DATASET} data-test="edit-dataset">
- {t('Edit dataset')}
+ <Menu.Item
+ key={EDIT_DATASET}
+ data-test="edit-dataset"
+ disabled={!allowEdit}
+ >
+ {!allowEdit ? (
+ <Tooltip
+ title={t(
+ 'You must be a dataset owner in order to edit. Please reach out to a dataset owner to request modifications or edit access.',
+ )}
+ >
+ {editText}
+ </Tooltip>
+ ) : (
+ editText
+ )}
</Menu.Item>
)}
<Menu.Item key={CHANGE_DATASET}>{t('Change dataset')}</Menu.Item>
diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.test.jsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.test.jsx
index 0854f006de..58acc6e32a 100644
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.test.jsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.test.jsx
@@ -55,6 +55,7 @@ const mockdatasets = [...new Array(3)].map((_, i) => ({
id: i,
schema: `schema ${i}`,
table_name: `coolest table ${i}`,
+ owners: [{ username: 'admin', userId: 1 }],
}));
const mockUser = {
diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
index c99a69e1c1..563a3bce1f 100644
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
@@ -56,7 +56,9 @@ import InfoTooltip from 'src/components/InfoTooltip';
import ImportModelsModal from 'src/components/ImportModal/index';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
+import { isUserAdmin } from 'src/dashboard/util/findPermission';
import AddDatasetModal from './AddDatasetModal';
+
import {
PAGE_SIZE,
SORT_BY,
@@ -75,6 +77,25 @@ const FlexRowContainer = styled.div`
const Actions = styled.div`
color: ${({ theme }) => theme.colors.grayscale.base};
+
+ .disabled {
+ svg,
+ i {
+ &:hover {
+ path {
+ fill: ${({ theme }) => theme.colors.grayscale.light1};
+ }
+ }
+ }
+ color: ${({ theme }) => theme.colors.grayscale.light1};
+ .ant-menu-item:hover {
+ color: ${({ theme }) => theme.colors.grayscale.light1};
+ cursor: default;
+ }
+ &::after {
+ color: ${({ theme }) => theme.colors.grayscale.light1};
+ }
+ }
`;
type Dataset = {
@@ -344,6 +365,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
},
{
Cell: ({ row: { original } }: any) => {
+ // Verify owner or isAdmin
+ const allowEdit =
+ original.owners.map((o: Owner) => o.id).includes(user.userId) ||
+ isUserAdmin(user);
+
const handleEdit = () => openDatasetEditModal(original);
const handleDelete = () => openDatasetDeleteModal(original);
const handleExport = () => handleBulkDatasetExport([original]);
@@ -387,14 +413,20 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
{canEdit && (
<Tooltip
id="edit-action-tooltip"
- title={t('Edit')}
- placement="bottom"
+ title={
+ allowEdit
+ ? t('Edit')
+ : t(
+ 'You must be a dataset owner in order to edit. Please reach out to a dataset owner to request modifications or edit access.',
+ )
+ }
+ placement="bottomRight"
>
<span
role="button"
tabIndex={0}
- className="action-button"
- onClick={handleEdit}
+ className={allowEdit ? 'action-button' : 'disabled'}
+ onClick={allowEdit ? handleEdit : undefined}
>
<Icons.EditAlt />
</span>
diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py
index 791f7c500e..560c12d6f1 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -27,7 +27,7 @@ from marshmallow import ValidationError
from sqlalchemy.exc import NoSuchTableError
from sqlalchemy.orm.exc import NoResultFound
-from superset import app, db, event_logger
+from superset import db, event_logger
from superset.commands.utils import populate_owners
from superset.connectors.connector_registry import ConnectorRegistry
from superset.connectors.sqla.utils import get_physical_table_metadata
@@ -81,29 +81,15 @@ class Datasource(BaseSupersetView):
if "owners" in datasource_dict and orm_datasource.owner_class is not None:
# Check ownership
- if app.config.get("OLD_API_CHECK_DATASET_OWNERSHIP"):
- # mimic the behavior of the new dataset command that
- # checks ownership and ensures that non-admins aren't locked out
- # of the object
- try:
- check_ownership(orm_datasource)
- except SupersetSecurityException as ex:
- raise DatasetForbiddenError() from ex
- user = security_manager.get_user_by_id(g.user.id)
- datasource_dict["owners"] = populate_owners(
- user, datasource_dict["owners"], default_to_user=False
- )
- else:
- # legacy behavior
- datasource_dict["owners"] = (
- db.session.query(orm_datasource.owner_class)
- .filter(
- orm_datasource.owner_class.id.in_(
- datasource_dict["owners"] or []
- )
- )
- .all()
- )
+ try:
+ check_ownership(orm_datasource)
+ except SupersetSecurityException as ex:
+ raise DatasetForbiddenError() from ex
+
+ user = security_manager.get_user_by_id(g.user.id)
+ datasource_dict["owners"] = populate_owners(
+ user, datasource_dict["owners"], default_to_user=False
+ )
duplicates = [
name
diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py
index 1428a20c48..6d46afa0a9 100644
--- a/tests/integration_tests/datasource_tests.py
+++ b/tests/integration_tests/datasource_tests.py
@@ -278,6 +278,7 @@ class TestDatasource(SupersetTestCase):
datasource_post = get_datasource_post()
datasource_post["id"] = tbl_id
+ datasource_post["owners"] = [1]
data = dict(data=json.dumps(datasource_post))
resp = self.get_json_resp("/datasource/save/", data)
for k in datasource_post:
@@ -299,11 +300,14 @@ class TestDatasource(SupersetTestCase):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_change_database(self):
self.login(username="admin")
+ admin_user = self.get_user("admin")
+
tbl = self.get_table(name="birth_names")
tbl_id = tbl.id
db_id = tbl.database_id
datasource_post = get_datasource_post()
datasource_post["id"] = tbl_id
+ datasource_post["owners"] = [admin_user.id]
new_db = self.create_fake_db()
datasource_post["database"]["id"] = new_db.id
@@ -318,10 +322,12 @@ class TestDatasource(SupersetTestCase):
def test_save_duplicate_key(self):
self.login(username="admin")
+ admin_user = self.get_user("admin")
tbl_id = self.get_table(name="birth_names").id
datasource_post = get_datasource_post()
datasource_post["id"] = tbl_id
+ datasource_post["owners"] = [admin_user.id]
datasource_post["columns"].extend(
[
{
@@ -346,10 +352,12 @@ class TestDatasource(SupersetTestCase):
def test_get_datasource(self):
self.login(username="admin")
+ admin_user = self.get_user("admin")
tbl = self.get_table(name="birth_names")
datasource_post = get_datasource_post()
datasource_post["id"] = tbl.id
+ datasource_post["owners"] = [admin_user.id]
data = dict(data=json.dumps(datasource_post))
self.get_json_resp("/datasource/save/", data)
url = f"/datasource/get/{tbl.type}/{tbl.id}/"
[superset] 08/10: fix: Allow dataset owners to see their datasets (#20135)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 562e3a769b083a365ad8a967cdc980021426f33b
Author: cccs-tom <59...@users.noreply.github.com>
AuthorDate: Wed May 25 04:58:15 2022 -0400
fix: Allow dataset owners to see their datasets (#20135)
(cherry picked from commit b0c6935f0600f111f06ae7ff05f7fa902e9ad252)
---
superset/views/base.py | 9 +++++++++
tests/integration_tests/datasets/api_tests.py | 21 +++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/superset/views/base.py b/superset/views/base.py
index 3024c4490d..e505af5300 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -622,10 +622,19 @@ class DatasourceFilter(BaseFilter): # pylint: disable=too-few-public-methods
return query
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
+ owner_ids_query = (
+ db.session.query(models.SqlaTable.id)
+ .join(models.SqlaTable.owners)
+ .filter(
+ security_manager.user_model.id
+ == security_manager.user_model.get_user_id()
+ )
+ )
return query.filter(
or_(
self.model.perm.in_(datasource_perms),
self.model.schema_perm.in_(schema_perms),
+ models.SqlaTable.id.in_(owner_ids_query),
)
)
diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py
index bcc3f88b9f..2031d64557 100644
--- a/tests/integration_tests/datasets/api_tests.py
+++ b/tests/integration_tests/datasets/api_tests.py
@@ -213,6 +213,27 @@ class TestDatasetApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
assert response["result"] == []
+ def test_get_dataset_list_gamma_owned(self):
+ """
+ Dataset API: Test get dataset list owned by gamma
+ """
+ main_db = get_main_database()
+ owned_dataset = self.insert_dataset(
+ "ab_user", [self.get_user("gamma").id], main_db
+ )
+
+ self.login(username="gamma")
+ uri = "api/v1/dataset/"
+ rv = self.get_assert_metric(uri, "get_list")
+ assert rv.status_code == 200
+ response = json.loads(rv.data.decode("utf-8"))
+
+ assert response["count"] == 1
+ assert response["result"][0]["table_name"] == "ab_user"
+
+ db.session.delete(owned_dataset)
+ db.session.commit()
+
def test_get_dataset_related_database_gamma(self):
"""
Dataset API: Test get dataset related databases gamma
[superset] 05/10: fix: Athena timestamp literal format (#19970)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bda9044bfd5e43c1ac8afecc9f138e5abcd7d8f1
Author: thinhnd2104 <90...@users.noreply.github.com>
AuthorDate: Mon May 16 13:50:20 2022 +0700
fix: Athena timestamp literal format (#19970)
* Fix Athena timestamp literal format
related: [#19969]
* Update test_athena.py
Update test
* Update athena.py
* Resolving CI/CD
* lint
Co-authored-by: Thinh Nguyen Duc <th...@tripi.vn>
Co-authored-by: Ville Brofeldt <vi...@gmail.com>
(cherry picked from commit 85315460cbc23cae6ada309012f6d9057f23019d)
---
superset/db_engine_specs/athena.py | 2 +-
tests/unit_tests/db_engine_specs/test_athena.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/superset/db_engine_specs/athena.py b/superset/db_engine_specs/athena.py
index 2394fe088d..9e1d798a7c 100644
--- a/superset/db_engine_specs/athena.py
+++ b/superset/db_engine_specs/athena.py
@@ -69,7 +69,7 @@ class AthenaEngineSpec(BaseEngineSpec):
if tt == utils.TemporalType.DATE:
return f"DATE '{dttm.date().isoformat()}'"
if tt == utils.TemporalType.TIMESTAMP:
- datetime_formatted = dttm.isoformat(timespec="microseconds")
+ datetime_formatted = dttm.isoformat(sep=" ", timespec="milliseconds")
return f"""TIMESTAMP '{datetime_formatted}'"""
return None
diff --git a/tests/unit_tests/db_engine_specs/test_athena.py b/tests/unit_tests/db_engine_specs/test_athena.py
index a35d8eddfb..d7c1a3f606 100644
--- a/tests/unit_tests/db_engine_specs/test_athena.py
+++ b/tests/unit_tests/db_engine_specs/test_athena.py
@@ -39,7 +39,7 @@ def test_convert_dttm(app_context: AppContext, dttm: datetime) -> None:
assert (
AthenaEngineSpec.convert_dttm("TIMESTAMP", dttm)
- == "TIMESTAMP '2019-01-02T03:04:05.678900'"
+ == "TIMESTAMP '2019-01-02 03:04:05.678'"
)