diff --git a/superset/charts/api.py b/superset/charts/api.py index be4f40747b..3c11cbab5e 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -56,7 +56,11 @@ 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.screenshots import ChartScreenshot -from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter +from superset.views.base_api import ( + BaseSupersetModelRestApi, + RelatedFieldFilter, + statsd_metrics, +) from superset.views.filters import FilterRelatedOwners logger = logging.getLogger(__name__) @@ -147,6 +151,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @expose("/", methods=["POST"]) @protect() @safe + @statsd_metrics def post(self) -> Response: """Creates a new Chart --- @@ -199,6 +204,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @expose("/", methods=["PUT"]) @protect() @safe + @statsd_metrics def put( # pylint: disable=too-many-return-statements, arguments-differ self, pk: int ) -> Response: @@ -266,6 +272,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @expose("/", methods=["DELETE"]) @protect() @safe + @statsd_metrics def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ """Deletes a Chart --- @@ -312,6 +319,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @expose("/", methods=["DELETE"]) @protect() @safe + @statsd_metrics @rison(get_delete_ids_schema) def bulk_delete( self, **kwargs: Any @@ -373,6 +381,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @event_logger.log_this @protect() @safe + @statsd_metrics def data(self) -> Response: """ Takes a query context constructed in the client and returns payload @@ -429,6 +438,7 @@ class ChartRestApi(BaseSupersetModelRestApi): @protect() @rison(thumbnail_query_schema) @safe + @statsd_metrics def thumbnail( self, pk: int, digest: str, **kwargs: Dict[str, bool] ) -> WerkzeugResponse: diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index 6a78f30eb6..0f08f4a654 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -97,20 +97,20 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_delete_chart(self): """ - Chart API: Test delete + Chart API: Test delete """ admin_id = self.get_user("admin").id chart_id = self.insert_chart("name", [admin_id], 1).id self.login(username="admin") uri = f"api/v1/chart/{chart_id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") self.assertEqual(rv.status_code, 200) model = db.session.query(Slice).get(chart_id) self.assertEqual(model, None) def test_delete_bulk_charts(self): """ - Chart API: Test delete bulk + Chart API: Test delete bulk """ admin_id = self.get_user("admin").id chart_count = 4 @@ -122,7 +122,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): self.login(username="admin") argument = chart_ids uri = f"api/v1/chart/?q={prison.dumps(argument)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 200) response = json.loads(rv.data.decode("utf-8")) expected_response = {"message": f"Deleted {chart_count} charts"} @@ -133,54 +133,54 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_delete_bulk_chart_bad_request(self): """ - Chart API: Test delete bulk bad request + Chart API: Test delete bulk bad request """ chart_ids = [1, "a"] self.login(username="admin") argument = chart_ids uri = f"api/v1/chart/?q={prison.dumps(argument)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 400) def test_delete_not_found_chart(self): """ - Chart API: Test not found delete + Chart API: Test not found delete """ self.login(username="admin") chart_id = 1000 uri = f"api/v1/chart/{chart_id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") self.assertEqual(rv.status_code, 404) def test_delete_bulk_charts_not_found(self): """ - Chart API: Test delete bulk not found + Chart API: Test delete bulk not found """ max_id = db.session.query(func.max(Slice.id)).scalar() chart_ids = [max_id + 1, max_id + 2] self.login(username="admin") argument = chart_ids uri = f"api/v1/chart/?q={prison.dumps(argument)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 404) def test_delete_chart_admin_not_owned(self): """ - Chart API: Test admin delete not owned + Chart API: Test admin delete not owned """ gamma_id = self.get_user("gamma").id chart_id = self.insert_chart("title", [gamma_id], 1).id self.login(username="admin") uri = f"api/v1/chart/{chart_id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") self.assertEqual(rv.status_code, 200) model = db.session.query(Slice).get(chart_id) self.assertEqual(model, None) def test_delete_bulk_chart_admin_not_owned(self): """ - Chart API: Test admin delete bulk not owned + Chart API: Test admin delete bulk not owned """ gamma_id = self.get_user("gamma").id chart_count = 4 @@ -193,7 +193,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): self.login(username="admin") argument = chart_ids uri = f"api/v1/chart/?q={prison.dumps(argument)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") response = json.loads(rv.data.decode("utf-8")) self.assertEqual(rv.status_code, 200) expected_response = {"message": f"Deleted {chart_count} charts"} @@ -205,7 +205,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_delete_chart_not_owned(self): """ - Chart API: Test delete try not owned + Chart API: Test delete try not owned """ user_alpha1 = self.create_user( "alpha1", "password", "Alpha", email="alpha1@superset.org" @@ -216,7 +216,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): chart = self.insert_chart("title", [user_alpha1.id], 1) self.login(username="alpha2", password="password") uri = f"api/v1/chart/{chart.id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") self.assertEqual(rv.status_code, 403) db.session.delete(chart) db.session.delete(user_alpha1) @@ -225,7 +225,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_delete_bulk_chart_not_owned(self): """ - Chart API: Test delete bulk try not owned + Chart API: Test delete bulk try not owned """ user_alpha1 = self.create_user( "alpha1", "password", "Alpha", email="alpha1@superset.org" @@ -248,7 +248,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): # verify we can't delete not owned charts arguments = [chart.id for chart in charts] uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 403) response = json.loads(rv.data.decode("utf-8")) expected_response = {"message": "Forbidden"} @@ -257,7 +257,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): # # nothing is deleted in bulk with a list of owned and not owned charts arguments = [chart.id for chart in charts] + [owned_chart.id] uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 403) response = json.loads(rv.data.decode("utf-8")) expected_response = {"message": "Forbidden"} @@ -272,7 +272,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_create_chart(self): """ - Chart API: Test create chart + Chart API: Test create chart """ admin_id = self.get_user("admin").id chart_data = { @@ -288,7 +288,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) model = db.session.query(Slice).get(data.get("id")) @@ -297,7 +297,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_create_simple_chart(self): """ - Chart API: Test create simple chart + Chart API: Test create simple chart """ chart_data = { "slice_name": "title1", @@ -306,7 +306,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) model = db.session.query(Slice).get(data.get("id")) @@ -315,7 +315,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_create_chart_validate_owners(self): """ - Chart API: Test create validate owners + Chart API: Test create validate owners """ chart_data = { "slice_name": "title1", @@ -325,7 +325,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) expected_response = {"message": {"owners": ["Owners are invalid"]}} @@ -333,7 +333,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_create_chart_validate_params(self): """ - Chart API: Test create validate params json + Chart API: Test create validate params json """ chart_data = { "slice_name": "title1", @@ -343,12 +343,12 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 400) def test_create_chart_validate_datasource(self): """ - Chart API: Test create validate datasource + Chart API: Test create validate datasource """ self.login(username="admin") chart_data = { @@ -357,7 +357,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): "datasource_type": "unknown", } uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( @@ -369,7 +369,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): "datasource_type": "table", } uri = f"api/v1/chart/" - rv = self.client.post(uri, json=chart_data) + rv = self.post_assert_metric(uri, chart_data, "post") self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( @@ -378,7 +378,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_update_chart(self): """ - Chart API: Test update + Chart API: Test update """ admin = self.get_user("admin") gamma = self.get_user("gamma") @@ -397,7 +397,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/{chart_id}" - rv = self.client.put(uri, json=chart_data) + rv = self.put_assert_metric(uri, chart_data, "put") self.assertEqual(rv.status_code, 200) model = db.session.query(Slice).get(chart_id) related_dashboard = db.session.query(Dashboard).get(1) @@ -417,7 +417,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_update_chart_new_owner(self): """ - Chart API: Test update set new owner to current user + Chart API: Test update set new owner to current user """ gamma = self.get_user("gamma") admin = self.get_user("admin") @@ -425,7 +425,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): chart_data = {"slice_name": "title1_changed"} self.login(username="admin") uri = f"api/v1/chart/{chart_id}" - rv = self.client.put(uri, json=chart_data) + rv = self.put_assert_metric(uri, chart_data, "put") self.assertEqual(rv.status_code, 200) model = db.session.query(Slice).get(chart_id) self.assertIn(admin, model.owners) @@ -434,7 +434,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_update_chart_not_owned(self): """ - Chart API: Test update not owned + Chart API: Test update not owned """ user_alpha1 = self.create_user( "alpha1", "password", "Alpha", email="alpha1@superset.org" @@ -447,7 +447,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): self.login(username="alpha2", password="password") chart_data = {"slice_name": "title1_changed"} uri = f"api/v1/chart/{chart.id}" - rv = self.client.put(uri, json=chart_data) + rv = self.put_assert_metric(uri, chart_data, "put") self.assertEqual(rv.status_code, 403) db.session.delete(chart) db.session.delete(user_alpha1) @@ -456,14 +456,14 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_update_chart_validate_datasource(self): """ - Chart API: Test update validate datasource + Chart API: Test update validate datasource """ admin = self.get_user("admin") chart = self.insert_chart("title", [admin.id], 1) self.login(username="admin") chart_data = {"datasource_id": 1, "datasource_type": "unknown"} uri = f"api/v1/chart/{chart.id}" - rv = self.client.put(uri, json=chart_data) + rv = self.put_assert_metric(uri, chart_data, "put") self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( @@ -471,7 +471,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): ) chart_data = {"datasource_id": 0, "datasource_type": "table"} uri = f"api/v1/chart/{chart.id}" - rv = self.client.put(uri, json=chart_data) + rv = self.put_assert_metric(uri, chart_data, "put") self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( @@ -482,7 +482,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_update_chart_validate_owners(self): """ - Chart API: Test update validate owners + Chart API: Test update validate owners """ chart_data = { "slice_name": "title1", @@ -500,13 +500,13 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_get_chart(self): """ - Chart API: Test get chart + Chart API: Test get chart """ admin = self.get_user("admin") chart = self.insert_chart("title", [admin.id], 1) self.login(username="admin") uri = f"api/v1/chart/{chart.id}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get") self.assertEqual(rv.status_code, 200) expected_result = { "cache_timeout": None, @@ -531,17 +531,17 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_get_chart_not_found(self): """ - Chart API: Test get chart not found + Chart API: Test get chart not found """ chart_id = 1000 self.login(username="admin") uri = f"api/v1/chart/{chart_id}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get") self.assertEqual(rv.status_code, 404) def test_get_chart_no_data_access(self): """ - Chart API: Test get chart without data access + Chart API: Test get chart without data access """ self.login(username="gamma") chart_no_access = ( @@ -555,30 +555,30 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_get_charts(self): """ - Chart API: Test get charts + Chart API: Test get charts """ self.login(username="admin") uri = f"api/v1/chart/" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 33) def test_get_charts_filter(self): """ - Chart API: Test get charts filter + Chart API: Test get charts filter """ self.login(username="admin") arguments = {"filters": [{"col": "slice_name", "opr": "sw", "value": "G"}]} uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 5) def test_get_charts_custom_filter(self): """ - Chart API: Test get charts custom filter + Chart API: Test get charts custom filter """ admin = self.get_user("admin") chart1 = self.insert_chart("foo", [admin.id], 1, description="ZY_bar") @@ -595,7 +595,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): } self.login(username="admin") uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 3) @@ -614,7 +614,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): self.logout() self.login(username="gamma") uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 0) @@ -628,7 +628,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_get_charts_page(self): """ - Chart API: Test get charts filter + Chart API: Test get charts filter """ # Assuming we have 33 sample charts self.login(username="admin") @@ -641,36 +641,37 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): arguments = {"page_size": 10, "page": 3} uri = f"api/v1/chart/?q={prison.dumps(arguments)}" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(len(data["result"]), 3) def test_get_charts_no_data_access(self): """ - Chart API: Test get charts no data access + Chart API: Test get charts no data access """ self.login(username="gamma") uri = f"api/v1/chart/" - rv = self.client.get(uri) + rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 0) def test_chart_data(self): """ - Query API: Test chart data query + Query API: Test chart data query """ self.login(username="admin") query_context = self._get_query_context() uri = "api/v1/chart/data" - rv = self.client.post(uri, json=query_context) + rv = self.post_assert_metric(uri, query_context, "data") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["result"][0]["rowcount"], 100) def test_invalid_chart_data(self): - """Query API: Test chart data query with invalid schema + """ + Query API: Test chart data query with invalid schema """ self.login(username="admin") query_context = self._get_query_context() @@ -681,10 +682,10 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): def test_query_exec_not_allowed(self): """ - Query API: Test chart data query not allowed + Query API: Test chart data query not allowed """ self.login(username="gamma") query_context = self._get_query_context() uri = "api/v1/chart/data" - rv = self.client.post(uri, json=query_context) + rv = self.post_assert_metric(uri, query_context, "data") self.assertEqual(rv.status_code, 401)