feat: finalize Word Cloud move to new chart data endpoint (#9975)

* remove word cloud from viz.py

* Fix Run in SQL Lab

* remove deprecated python tests

* break out legacy endpoint type into function

* Break out exploreChart from exportChart and implement results type

* Fix jest tests and refactor accordingly

* lint

* Rename v1 payload function

* Add dashboard id to v1 chart data request url params

* Add support for domain sharding to v1 chart data request
This commit is contained in:
Ville Brofeldt 2020-06-05 14:08:46 +03:00 committed by GitHub
parent 619fbc9557
commit 5c4d4f16b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 177 additions and 198 deletions

View File

@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import URI from 'urijs';
import fetchMock from 'fetch-mock';
import sinon from 'sinon';
@ -25,17 +26,16 @@ import * as exploreUtils from 'src/explore/exploreUtils';
import * as actions from 'src/chart/chartAction';
describe('chart actions', () => {
const V1_URL = '/http//localhost/api/v1/chart/data';
const MOCK_URL = '/mockURL';
let dispatch;
let urlStub;
let getExploreUrlStub;
let getChartDataUriStub;
let metadataRegistryStub;
let buildQueryRegistryStub;
let fakeMetadata;
const setupDefaultFetchMock = () => {
fetchMock.post(MOCK_URL, { json: {} }, { overwriteRoutes: true });
fetchMock.post(V1_URL, { json: {} }, { overwriteRoutes: true });
};
beforeAll(() => {
@ -46,9 +46,12 @@ describe('chart actions', () => {
beforeEach(() => {
dispatch = sinon.spy();
urlStub = sinon
getExploreUrlStub = sinon
.stub(exploreUtils, 'getExploreUrl')
.callsFake(() => MOCK_URL);
getChartDataUriStub = sinon
.stub(exploreUtils, 'getChartDataUri')
.callsFake(() => URI(MOCK_URL));
fakeMetadata = { useLegacyApi: true };
metadataRegistryStub = sinon
.stub(chartlib, 'getChartMetadataRegistry')
@ -65,7 +68,8 @@ describe('chart actions', () => {
});
afterEach(() => {
urlStub.restore();
getExploreUrlStub.restore();
getChartDataUriStub.restore();
fetchMock.resetHistory();
metadataRegistryStub.restore();
buildQueryRegistryStub.restore();
@ -80,8 +84,8 @@ describe('chart actions', () => {
const actionThunk = actions.postChartFormData({});
await actionThunk(dispatch);
expect(fetchMock.calls(V1_URL)).toHaveLength(1);
expect(fetchMock.calls(V1_URL)[0][1].body).toBe(
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
expect(fetchMock.calls(MOCK_URL)[0][1].body).toBe(
JSON.stringify({
some_param: 'fake query!',
result_type: 'full',

View File

@ -179,12 +179,14 @@ describe('ExploreResultsButton', () => {
beforeEach(() => {
sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => 'mockURL');
sinon.spy(exploreUtils, 'exportChart');
sinon.spy(exploreUtils, 'exploreChart');
sinon
.stub(wrapper.instance(), 'buildVizOptions')
.callsFake(() => mockOptions);
});
afterEach(() => {
exploreUtils.getExploreUrl.restore();
exploreUtils.exploreChart.restore();
exploreUtils.exportChart.restore();
wrapper.instance().buildVizOptions.restore();
fetchMock.reset();
@ -224,8 +226,8 @@ describe('ExploreResultsButton', () => {
setTimeout(() => {
expect(datasourceSpy.callCount).toBe(1);
expect(exploreUtils.exportChart.callCount).toBe(1);
expect(exploreUtils.exportChart.getCall(0).args[0].datasource).toBe(
expect(exploreUtils.exploreChart.callCount).toBe(1);
expect(exploreUtils.exploreChart.getCall(0).args[0].datasource).toBe(
'107__table',
);
expect(infoToastSpy.callCount).toBe(1);

View File

@ -77,7 +77,7 @@ class ExploreCtasResultsButton extends React.PureComponent {
);
// open new window for data visualization
exportChart(formData);
exportChart({ formData });
})
.catch(() => {
this.props.actions.addDangerToast(

View File

@ -27,7 +27,7 @@ import { t } from '@superset-ui/translation';
import { InfoTooltipWithTrigger } from '@superset-ui/control-utils';
import shortid from 'shortid';
import { exportChart } from '../../explore/exploreUtils';
import { exploreChart } from '../../explore/exploreUtils';
import * as actions from '../actions/sqlLab';
import Button from '../../components/Button';
@ -150,7 +150,7 @@ class ExploreResultsButton extends React.PureComponent {
);
// open new window for data visualization
exportChart(formData);
exploreChart(formData);
})
.catch(() => {
this.props.actions.addDangerToast(

View File

@ -18,18 +18,20 @@
*/
/* eslint no-undef: 'error' */
/* eslint no-param-reassign: ["error", { "props": false }] */
import URI from 'urijs';
import moment from 'moment';
import { t } from '@superset-ui/translation';
import { SupersetClient } from '@superset-ui/connection';
import {
getChartBuildQueryRegistry,
getChartMetadataRegistry,
} from '@superset-ui/chart';
import { isFeatureEnabled, FeatureFlag } from '../featureFlags';
import {
getExploreUrl,
getAnnotationJsonUrl,
getExploreUrl,
getHostName,
getLegacyEndpointType,
buildV1ChartDataPayload,
postForm,
shouldUseLegacyApi,
getChartDataUri,
} from '../explore/exploreUtils';
import {
requiresQuery,
@ -102,11 +104,6 @@ export function annotationQueryFailed(annotation, queryResponse, key) {
return { type: ANNOTATION_QUERY_FAILED, annotation, queryResponse, key };
}
const shouldUseLegacyApi = formData => {
const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type);
return useLegacyApi || false;
};
const legacyChartDataRequest = async (
formData,
resultFormat,
@ -115,11 +112,7 @@ const legacyChartDataRequest = async (
method = 'POST',
requestParams = {},
) => {
const endpointType = ['base', 'csv', 'results', 'samples'].includes(
resultType,
)
? resultType
: resultFormat;
const endpointType = getLegacyEndpointType({ resultFormat, resultType });
const url = getExploreUrl({
formData,
endpointType,
@ -156,17 +149,24 @@ const v1ChartDataRequest = async (
force,
requestParams,
) => {
const buildQuery = await getChartBuildQueryRegistry().get(formData.viz_type);
const payload = buildQuery({
...formData,
force,
});
const payload = buildV1ChartDataPayload({ formData, force });
// TODO: remove once these are added to superset-ui/query
payload.result_type = resultType;
payload.result_format = resultFormat;
// The dashboard id is added to query params for tracking purposes
const qs = requestParams.dashboard_id
? { dashboard_id: requestParams.dashboard_id }
: {};
const url = getChartDataUri({
path: '/api/v1/chart/data',
qs,
allowDomainSharding,
}).toString();
const querySettings = {
...requestParams,
endpoint: '/api/v1/chart/data',
url,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
};
@ -175,14 +175,14 @@ const v1ChartDataRequest = async (
});
};
export async function getChartDataRequest(
export async function getChartDataRequest({
formData,
resultFormat = 'json',
resultType = 'full',
force = false,
method = 'POST',
requestParams = {},
) {
}) {
let querySettings = {
...requestParams,
};
@ -337,14 +337,14 @@ export function exploreJSON(
};
if (dashboardId) requestParams.dashboard_id = dashboardId;
const chartDataRequest = getChartDataRequest(
const chartDataRequest = getChartDataRequest({
formData,
'json',
'full',
resultFormat: 'json',
resultType: 'full',
force,
method,
requestParams,
);
});
dispatch(chartUpdateStarted(controller, formData, key));
@ -460,19 +460,12 @@ export function postChartFormData(
export function redirectSQLLab(formData) {
return dispatch => {
const url = getExploreUrl({
formData,
endpointType: 'query',
});
return SupersetClient.post({
url,
postPayload: { form_data: formData },
})
.then(({ json }) => {
getChartDataRequest({ formData, resultFormat: 'json', resultType: 'query' })
.then(({ result }) => {
const redirectUrl = '/superset/sqllab';
const payload = {
datasourceKey: formData.datasource,
sql: json.query,
sql: result[0].query,
};
postForm(redirectUrl, payload);
})

View File

@ -19,7 +19,7 @@
import cx from 'classnames';
import React from 'react';
import PropTypes from 'prop-types';
import { exportChart } from '../../../explore/exploreUtils';
import { exploreChart, exportChart } from '../../../explore/exploreUtils';
import SliceHeader from '../SliceHeader';
import ChartContainer from '../../../chart/ChartContainer';
import MissingChart from '../MissingChart';
@ -191,7 +191,7 @@ class Chart extends React.Component {
slice_id: this.props.slice.slice_id,
is_cached: this.props.isCached,
});
exportChart(this.props.formData);
exploreChart(this.props.formData);
}
exportCSV() {
@ -199,7 +199,11 @@ class Chart extends React.Component {
slice_id: this.props.slice.slice_id,
is_cached: this.props.isCached,
});
exportChart(this.props.formData, 'csv');
exportChart({
formData: this.props.formData,
resultType: 'results',
resultFormat: 'csv',
});
}
forceRefresh() {

View File

@ -90,7 +90,11 @@ export class DisplayQueryButton extends React.PureComponent {
beforeOpen(resultType) {
this.setState({ isLoading: true });
getChartDataRequest(this.props.latestQueryFormData, 'json', resultType)
getChartDataRequest({
formData: this.props.latestQueryFormData,
resultFormat: 'json',
resultType,
})
.then(response => {
// Currently displaying of only first query is supported
const result = response.result[0];

View File

@ -47,8 +47,16 @@ export default function ExploreActionButtons({
const exportToCSVClasses = cx('btn btn-default btn-sm', {
'disabled disabledButton': !canDownload,
});
const doExportCSV = exportChart.bind(this, latestQueryFormData, 'csv');
const doExportChart = exportChart.bind(this, latestQueryFormData, 'results');
const doExportCSV = exportChart.bind(this, {
formData: latestQueryFormData,
resultType: 'results',
resultFormat: 'csv',
});
const doExportChart = exportChart.bind(this, {
formData: latestQueryFormData,
resultType: 'results',
resultFormat: 'json',
});
return (
<div className="btn-group results" role="group">

View File

@ -19,8 +19,12 @@
/* eslint camelcase: 0 */
import URI from 'urijs';
import { SupersetClient } from '@superset-ui/connection';
import { allowCrossDomain, availableDomains } from '../utils/hostNamesConfig';
import { safeStringify } from '../utils/safeStringify';
import { allowCrossDomain, availableDomains } from 'src/utils/hostNamesConfig';
import { safeStringify } from 'src/utils/safeStringify';
import {
getChartBuildQueryRegistry,
getChartMetadataRegistry,
} from '@superset-ui/chart';
const MAX_URL_LENGTH = 8000;
@ -30,7 +34,7 @@ export function getChartKey(explore) {
}
let requestCounter = 0;
function getHostName(allowDomainSharding = false) {
export function getHostName(allowDomainSharding = false) {
let currentIndex = 0;
if (allowDomainSharding) {
currentIndex = requestCounter % availableDomains.length;
@ -44,7 +48,6 @@ function getHostName(allowDomainSharding = false) {
requestCounter += 1;
}
}
return availableDomains[currentIndex];
}
@ -109,6 +112,22 @@ export function getExploreLongUrl(
return url;
}
export function getChartDataUri({ path, qs, allowDomainSharding = false }) {
// The search params from the window.location are carried through,
// but can be specified with curUrl (used for unit tests to spoof
// the window.location).
let uri = new URI({
protocol: location.protocol.slice(0, -1),
hostname: getHostName(allowDomainSharding),
port: location.port ? location.port : '',
path,
});
if (qs) {
uri = uri.search(qs);
}
return uri;
}
export function getExploreUrl({
formData,
endpointType = 'base',
@ -121,17 +140,7 @@ export function getExploreUrl({
if (!formData.datasource) {
return null;
}
// The search params from the window.location are carried through,
// but can be specified with curUrl (used for unit tests to spoof
// the window.location).
let uri = new URI({
protocol: location.protocol.slice(0, -1),
hostname: getHostName(allowDomainSharding),
port: location.port ? location.port : '',
path: '/',
});
let uri = getChartDataUri({ path: '/', allowDomainSharding });
if (curUrl) {
uri = URI(URI(curUrl).search());
}
@ -183,6 +192,23 @@ export function getExploreUrl({
return uri.search(search).directory(directory).toString();
}
export const shouldUseLegacyApi = formData => {
const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type);
return useLegacyApi || false;
};
export const buildV1ChartDataPayload = ({ formData, force }) => {
const buildQuery = getChartBuildQueryRegistry().get(formData.viz_type);
return buildQuery({
...formData,
force,
});
};
export const getLegacyEndpointType = ({ resultType, resultFormat }) => {
return resultFormat === 'csv' ? resultFormat : resultType;
};
export function postForm(url, payload, target = '_blank') {
if (!url) {
return;
@ -208,11 +234,40 @@ export function postForm(url, payload, target = '_blank') {
document.body.removeChild(hiddenForm);
}
export function exportChart(formData, endpointType) {
export const exportChart = ({
formData,
resultFormat = 'json',
resultType = 'full',
force = false,
}) => {
let url;
let payload;
if (shouldUseLegacyApi(formData)) {
const endpointType = getLegacyEndpointType({ resultFormat, resultType });
url = getExploreUrl({
formData,
endpointType,
allowDomainSharding: false,
});
payload = formData;
} else {
url = '/api/v1/chart/data';
const buildQuery = getChartBuildQueryRegistry().get(formData.viz_type);
payload = buildQuery({
...formData,
force,
});
payload.result_type = resultType;
payload.result_format = resultFormat;
}
postForm(url, payload);
};
export const exploreChart = formData => {
const url = getExploreUrl({
formData,
endpointType,
endpointType: 'base',
allowDomainSharding: false,
});
postForm(url, formData);
}
};

View File

@ -24,7 +24,7 @@ import Dialog from 'react-bootstrap-dialog';
import { t } from '@superset-ui/translation';
import { InfoTooltipWithTrigger } from '@superset-ui/control-utils';
import { exportChart } from '../../explore/exploreUtils';
import { exploreChart } from '../../explore/exploreUtils';
import * as actions from '../actions/sqlLab';
import Button from '../../components/Button';
@ -76,7 +76,7 @@ class ExploreCtasResultsButton extends React.PureComponent {
);
// open new window for data visualization
exportChart(formData);
exploreChart(formData);
})
.catch(() => {
this.props.actions.addDangerToast(

View File

@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import json
import logging
from typing import Any, Dict
@ -55,13 +56,14 @@ from superset.exceptions import SupersetSecurityException
from superset.extensions import event_logger, security_manager
from superset.models.slice import Slice
from superset.tasks.thumbnails import cache_chart_thumbnail
from superset.utils.core import json_int_dttm_ser
from superset.utils.core import ChartDataResultFormat, json_int_dttm_ser
from superset.utils.screenshots import ChartScreenshot
from superset.views.base_api import (
BaseSupersetModelRestApi,
RelatedFieldFilter,
statsd_metrics,
)
from superset.views.core import CsvResponse, generate_download_headers
from superset.views.filters import FilterRelatedOwners
logger = logging.getLogger(__name__)
@ -431,10 +433,16 @@ class ChartRestApi(BaseSupersetModelRestApi):
500:
$ref: '#/components/responses/500'
"""
if not request.is_json:
if request.is_json:
json_body = request.json
elif request.form.get("form_data"):
# CSV export submits regular form data
json_body = json.loads(request.form["form_data"])
else:
return self.response_400(message="Request is not JSON")
try:
query_context, errors = ChartDataQueryContextSchema().load(request.json)
query_context, errors = ChartDataQueryContextSchema().load(json_body)
if errors:
return self.response_400(
message=_("Request is incorrect: %(error)s", error=errors)
@ -445,13 +453,25 @@ class ChartRestApi(BaseSupersetModelRestApi):
security_manager.assert_query_context_permission(query_context)
except SupersetSecurityException:
return self.response_401()
payload_json = query_context.get_payload()
response_data = simplejson.dumps(
{"result": payload_json}, default=json_int_dttm_ser, ignore_nan=True
)
resp = make_response(response_data, 200)
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
payload = query_context.get_payload()
result_format = query_context.result_format
if result_format == ChartDataResultFormat.CSV:
# return the first result
result = payload[0]["data"]
return CsvResponse(
result,
status=200,
headers=generate_download_headers("csv"),
mimetype="application/csv",
)
elif result_format == ChartDataResultFormat.JSON:
response_data = simplejson.dumps(
{"result": payload}, default=json_int_dttm_ser, ignore_nan=True
)
resp = make_response(response_data, 200)
resp.headers["Content-Type"] = "application/json; charset=utf-8"
return resp
raise self.response_400(message=f"Unsupported result_format: {result_format}")
@expose("/<pk>/thumbnail/<digest>/", methods=["GET"])
@protect()

View File

@ -157,7 +157,6 @@ class QueryContext:
query_obj.post_processing = []
query_obj.row_limit = min(row_limit, config["SAMPLES_ROW_LIMIT"])
query_obj.columns = [o.column_name for o in self.datasource.columns]
payload = self.get_df_payload(query_obj)
df = payload["df"]
status = payload["status"]
@ -167,6 +166,8 @@ class QueryContext:
else:
payload["data"] = self.get_data(df)
del payload["df"]
if self.result_type == utils.ChartDataResultType.RESULTS:
return {"data": payload["data"]}
return payload
def get_payload(self) -> List[Dict[str, Any]]:

View File

@ -824,24 +824,6 @@ class SeparatorViz(MarkupViz):
verbose_name = _("Separator")
class WordCloudViz(BaseViz):
"""Build a colorful word cloud
Uses the nice library at:
https://github.com/jasondavies/d3-cloud
"""
viz_type = "word_cloud"
verbose_name = _("Word Cloud")
is_timeseries = False
def query_obj(self) -> QueryObjectDict:
d = super().query_obj()
d["groupby"] = [self.form_data.get("series")]
return d
class TreemapViz(BaseViz):
"""Tree map visualisation for hierarchical data."""

View File

@ -860,19 +860,6 @@ class SeparatorViz(MarkupViz):
verbose_name = _("Separator")
class WordCloudViz(BaseViz):
"""Build a colorful word cloud
Uses the nice library at:
https://github.com/jasondavies/d3-cloud
"""
viz_type = "word_cloud"
verbose_name = _("Word Cloud")
is_timeseries = False
class TreemapViz(BaseViz):
"""Tree map visualisation for hierarchical data."""

View File

@ -161,24 +161,6 @@ class CoreTests(SupersetTestCase):
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)
def test_old_slice_json_endpoint(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
json_endpoint = "/superset/explore_json/{}/{}/".format(
slc.datasource_type, slc.datasource_id
)
resp = self.get_resp(
json_endpoint, {"form_data": json.dumps(slc.viz.form_data)}
)
assert '"Jennifer"' in resp
def test_slice_json_endpoint(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
resp = self.get_resp(slc.explore_json_url)
assert '"Jennifer"' in resp
def test_annotation_json_endpoint(self):
# Set up an annotation layer and annotation
layer = AnnotationLayer(name="foo", descr="bar")
@ -202,26 +184,6 @@ class CoreTests(SupersetTestCase):
assert "my_annotation" in resp
def test_old_slice_csv_endpoint(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
csv_endpoint = "/superset/explore_json/{}/{}/?csv=true".format(
slc.datasource_type, slc.datasource_id
)
resp = self.get_resp(csv_endpoint, {"form_data": json.dumps(slc.viz.form_data)})
assert "Jennifer," in resp
def test_slice_csv_endpoint(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
csv_endpoint = "/superset/explore_json/?csv=true"
resp = self.get_resp(
csv_endpoint, {"form_data": json.dumps({"slice_id": slc.id})}
)
assert "Jennifer," in resp
def test_admin_only_permissions(self):
def assert_admin_permission_in(role_name, assert_func):
role = security_manager.find_role(role_name)
@ -348,7 +310,6 @@ class CoreTests(SupersetTestCase):
for slc in db.session.query(Slc).all():
urls += [
(slc.slice_name, "explore", slc.slice_url),
(slc.slice_name, "explore_json", slc.explore_json_url),
]
for name, method, url in urls:
logger.info(f"[{name}]/[{method}]: {url}")
@ -788,15 +749,6 @@ class CoreTests(SupersetTestCase):
self.get_resp(slc.slice_url, {"form_data": json.dumps(slc.form_data)})
self.assertEqual(1, qry.count())
def test_slice_id_is_always_logged_correctly_on_ajax_request(self):
# superset/explore_json case
self.login(username="admin")
slc = db.session.query(Slice).filter_by(slice_name="Girls").one()
qry = db.session.query(models.Log).filter_by(slice_id=slc.id)
slc_url = slc.slice_url.replace("explore", "explore_json")
self.get_json_resp(slc_url, {"form_data": json.dumps(slc.form_data)})
self.assertEqual(1, qry.count())
def create_sample_csvfile(self, filename: str, content: List[str]) -> None:
with open(filename, "w+") as test_file:
for l in content:
@ -1002,39 +954,6 @@ class CoreTests(SupersetTestCase):
rendered_query = str(table.get_from_clause())
self.assertEqual(clean_query, rendered_query)
def test_slice_payload_no_results(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
json_endpoint = "/superset/explore_json/"
form_data = slc.form_data
form_data.update(
{
"adhoc_filters": [
{
"clause": "WHERE",
"comparator": "NA",
"expressionType": "SIMPLE",
"operator": "==",
"subject": "gender",
}
]
}
)
data = self.get_json_resp(json_endpoint, {"form_data": json.dumps(form_data)})
self.assertEqual(data["status"], utils.QueryStatus.SUCCESS)
self.assertEqual(data["errors"], [])
def test_slice_payload_invalid_query(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
form_data = slc.form_data
form_data.update({"groupby": ["N/A"]})
data = self.get_json_resp(
"/superset/explore_json/", {"form_data": json.dumps(form_data)}
)
self.assertEqual(data["status"], utils.QueryStatus.FAILED)
def test_slice_payload_no_datasource(self):
self.login(username="admin")
data = self.get_json_resp("/superset/explore_json/", raise_on_error=False)