From d493bb89d607faf8a59aeaaff23ad621a472f8df Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Thu, 15 Apr 2021 15:55:11 +0300 Subject: [PATCH] add migration test --- superset/constants.py | 12 +- ...ff221_migrate_filter_sets_to_new_format.py | 177 +++++---- tests/migrations/__init__.py | 16 + tests/migrations/fc3a3a8ff221_tests.py | 364 ++++++++++++++++++ 4 files changed, 495 insertions(+), 74 deletions(-) create mode 100644 tests/migrations/__init__.py create mode 100644 tests/migrations/fc3a3a8ff221_tests.py diff --git a/superset/constants.py b/superset/constants.py index 215eb057f6bae..0c9016fb55356 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -118,14 +118,14 @@ class RouteMethod: # pylint: disable=too-few-public-methods "get_datasets": "read", } -EXTRA_FORM_DATA_APPEND_KEYS = [ +EXTRA_FORM_DATA_APPEND_KEYS = { "adhoc_filters", "filters", "interactive_groupby", "interactive_highlight", "interactive_drilldown", "custom_form_data", -] +} EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS = { "granularity": "granularity", @@ -135,15 +135,15 @@ class RouteMethod: # pylint: disable=too-few-public-methods "time_range": "time_range", } -EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS = [ +EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS = { "druid_time_origin", "relative_start", "relative_end", "time_grain_sqla", "time_range_endpoints", -] +} EXTRA_FORM_DATA_OVERRIDE_KEYS = ( - list(EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.values()) - + EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS + set(EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.values()) + | EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS ) diff --git a/superset/migrations/versions/fc3a3a8ff221_migrate_filter_sets_to_new_format.py b/superset/migrations/versions/fc3a3a8ff221_migrate_filter_sets_to_new_format.py index 7685f60afe2c5..79b032894f058 100644 --- a/superset/migrations/versions/fc3a3a8ff221_migrate_filter_sets_to_new_format.py +++ b/superset/migrations/versions/fc3a3a8ff221_migrate_filter_sets_to_new_format.py @@ -27,7 +27,7 @@ down_revision = "085f06488938" import json -from typing import Any, Dict, List +from typing import Any, Dict, Iterable from alembic import op from sqlalchemy import Column, Integer, Text @@ -47,30 +47,40 @@ class Dashboard(Base): # these are copied over from `superset/constants.py` to make sure they stay unchanged -EXTRA_FORM_DATA_APPEND_KEYS = [ +EXTRA_FORM_DATA_APPEND_KEYS = { "adhoc_filters", "filters", "interactive_groupby", "interactive_highlight", "interactive_drilldown", "custom_form_data", -] +} -EXTRA_FORM_DATA_OVERRIDE_KEYS = [ - "druid_time_origin", - "relative_start", - "relative_end", - "time_grain_sqla", - "time_range_endpoints", +EXTRA_FORM_DATA_OVERRIDE_REGULAR_KEYS = { "granularity", "granularity_sqla", "time_column", "time_grain", "time_range", -] +} +EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS = { + "druid_time_origin", + "relative_start", + "relative_end", + "time_grain_sqla", + "time_range_endpoints", +} -def _update_select_filters(native_filters: List[Dict[str, Any]]) -> None: +EXTRA_FORM_DATA_OVERRIDE_KEYS = ( + EXTRA_FORM_DATA_OVERRIDE_REGULAR_KEYS | EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS +) + + +def upgrade_select_filters(native_filters: Iterable[Dict[str, Any]]) -> None: + """ + Add `defaultToFirstItem` to `controlValues` of `select_filter` components + """ for native_filter in native_filters: filter_type = native_filter.get("filterType") if filter_type == "filter_select": @@ -79,6 +89,86 @@ def _update_select_filters(native_filters: List[Dict[str, Any]]) -> None: control_values["defaultToFirstItem"] = value +def upgrade_filter_set(filter_set: Dict[str, Any]) -> int: + changed_filters = 0 + upgrade_select_filters(filter_set.get("nativeFilters", {}).values()) + data_mask = filter_set.get("dataMask", {}) + native_filters = data_mask.pop("nativeFilters", {}) + for filter_id, filter_obj in native_filters.items(): + changed_filters += 1 + # move filter up one level + data_mask[filter_id] = filter_obj + + # rename currentState to filterState + current_state = filter_obj.pop("currentState", {}) + filter_obj["filterState"] = current_state + + # create new extraFormData field + old_extra_form_data = filter_obj.pop("extraFormData", {}) + extra_form_data = {} + filter_obj["extraFormData"] = extra_form_data + + # upgrade append filters + appends = old_extra_form_data.pop("append_form_data", {}) + extra_form_data.update(appends) + + # upgrade override filters + overrides = old_extra_form_data.pop("override_form_data", {}) + for override_key, override_value in overrides.items(): + # nested extras are also moved up to main object + if override_key == "extras": + for extra_key, extra_value in override_value.items(): + extra_form_data[extra_key] = extra_value + else: + extra_form_data[override_key] = override_value + return changed_filters + + +def downgrade_filter_set(filter_set: Dict[str, Any]) -> int: + changed_filters = 0 + old_data_mask = filter_set.pop("dataMask", {}) + native_filters = {} + data_mask = {"nativeFilters": native_filters} + filter_set["dataMask"] = data_mask + for filter_id, filter_obj in old_data_mask.items(): + changed_filters += 1 + # move filter object down one level + native_filters[filter_id] = filter_obj + + # downgrade filter state + filter_state = filter_obj.pop("filterState", {}) + filter_obj["currentState"] = filter_state + + old_extra_form_data = filter_obj.pop("extraFormData", {}) + extra_form_data = {} + filter_obj["extraFormData"] = extra_form_data + + # downgrade append keys + append_form_data = {} + extra_form_data["append_form_data"] = append_form_data + for key in EXTRA_FORM_DATA_APPEND_KEYS: + value = old_extra_form_data.pop(key, None) + if value is not None: + append_form_data[key] = value + if not append_form_data: + del extra_form_data["append_form_data"] + + # downgrade override keys + override_form_data = {} + extra_form_data["override_form_data"] = override_form_data + for key in EXTRA_FORM_DATA_OVERRIDE_KEYS: + value = old_extra_form_data.pop(key, None) + if key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS: + extras = override_form_data.get("extras", {}) + extras[key] = value + elif value is not None: + override_form_data[key] = value + if not override_form_data: + del extra_form_data["override_form_data"] + + return changed_filters + + def upgrade(): bind = op.get_bind() session = db.Session(bind=bind) @@ -94,40 +184,21 @@ def upgrade(): json_metadata = json.loads(dashboard.json_metadata) # upgrade native select filter metadata - native_filters = json_metadata.get("native_filter_configuration", []) - _update_select_filters(native_filters) + native_filters = json_metadata.get("native_filter_configuration") + if native_filters: + upgrade_select_filters(native_filters) # upgrade filter sets - filter_sets = json_metadata.get("filter_sets_configuration", {}) - json_metadata["filter_sets_configuration"] = filter_sets + filter_sets = json_metadata["filter_sets_configuration"] for filter_set in filter_sets: - # first migrate filters that were created prior to first item option - _update_select_filters(filter_set.get("nativeFilters", {}).values()) changed_filter_sets += 1 - data_mask = filter_set.get("dataMask", {}) - native_filters = data_mask.pop("nativeFilters", {}) - for filter_id, filter_obj in native_filters.items(): - changed_filters += 1 - data_mask[filter_id] = filter_obj - - # upgrade append filters - appends = filter_obj.pop("append_form_data", {}) - filter_obj.update(appends) - - # upgrade override filters - overrides = filter_obj.pop("override_form_data", {}) - filter_obj.update(overrides) - - # upgrade filter state - current_state = filter_obj.pop("currentState", {}) - filter_obj["filterState"] = current_state + changed_filters += upgrade_filter_set(filter_set) dashboard.json_metadata = json.dumps(json_metadata, sort_keys=True) except Exception as e: - print(e) print(f"Parsing json_metadata for dashboard {dashboard.id} failed.") - pass + raise e session.commit() session.close() @@ -151,41 +222,11 @@ def downgrade(): json_metadata["filter_sets_configuration"] = filter_sets for filter_set in filter_sets: changed_filter_sets += 1 - old_data_mask = filter_set.pop("dataMask", {}) - native_filters = {} - data_mask = {"nativeFilters": native_filters} - filter_set["dataMask"] = data_mask - for filter_id, filter_obj in old_data_mask.items(): - changed_filters += 1 - native_filters[filter_id] = filter_obj - # downgrade filter state - filter_state = filter_obj.pop("filterState", {}) - filter_obj["currentState"] = filter_state - - # downgrade append keys - append_form_data = {} - filter_obj["append_form_data"] = append_form_data - for key in EXTRA_FORM_DATA_APPEND_KEYS: - value = filter_obj.pop(key, None) - if value is not None: - append_form_data[key] = value - if not append_form_data: - del filter_obj["append_form_data"] - - # downgrade override keys - override_form_data = {} - filter_obj["override_form_data"] = override_form_data - for key in EXTRA_FORM_DATA_OVERRIDE_KEYS: - value = filter_obj.pop(key, None) - if value is not None: - override_form_data[key] = value - if not override_form_data: - del filter_obj["override_form_data"] + changed_filters += downgrade_filter_set(filter_set) dashboard.json_metadata = json.dumps(json_metadata, sort_keys=True) except Exception as e: - print(e) print(f"Parsing json_metadata for dashboard {dashboard.id} failed.") - pass + raise e session.commit() session.close() diff --git a/tests/migrations/__init__.py b/tests/migrations/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/migrations/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/migrations/fc3a3a8ff221_tests.py b/tests/migrations/fc3a3a8ff221_tests.py new file mode 100644 index 0000000000000..5a5e0cc71e6bb --- /dev/null +++ b/tests/migrations/fc3a3a8ff221_tests.py @@ -0,0 +1,364 @@ +# 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 copy import deepcopy + +from superset.migrations.versions.fc3a3a8ff221_migrate_filter_sets_to_new_format import ( + downgrade_filter_set, + upgrade_filter_set, + upgrade_select_filters, +) + +native_filters_v1 = [ + { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-CZpnK0rM-", + "isInstant": True, + "name": "Region", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "region"}, "datasetId": 2}], + }, + { + "cascadeParentIds": [], + "defaultValue": "No filter", + "filterType": "filter_time", + "id": "NATIVE_FILTER-gCMse9C7e", + "isInstant": True, + "name": "Time Range", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{}], + }, + { + "cascadeParentIds": ["NATIVE_FILTER-CZpnK0rM-"], + "controlValues": { + "defaultToFirstItem": False, + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-oQRgQ25Au", + "isInstant": True, + "name": "Country", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "country_name"}, "datasetId": 2}], + }, +] +native_filters_v2 = [ + { + "cascadeParentIds": [], + "controlValues": { + "defaultToFirstItem": False, + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-CZpnK0rM-", + "isInstant": True, + "name": "Region", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "region"}, "datasetId": 2}], + }, + { + "cascadeParentIds": [], + "defaultValue": "No filter", + "filterType": "filter_time", + "id": "NATIVE_FILTER-gCMse9C7e", + "isInstant": True, + "name": "Time Range", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{}], + }, + { + "cascadeParentIds": ["NATIVE_FILTER-CZpnK0rM-"], + "controlValues": { + "defaultToFirstItem": False, + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-oQRgQ25Au", + "isInstant": True, + "name": "Country", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "country_name"}, "datasetId": 2}], + }, +] + +filter_sets_v1 = { + "name": "New filter set", + "id": "FILTERS_SET-tt_Ovwy95", + "nativeFilters": { + "NATIVE_FILTER-tx05Ze2Hm": { + "id": "NATIVE_FILTER-tx05Ze2Hm", + "name": "Time range", + "filterType": "filter_time", + "targets": [{}], + "defaultValue": "No filter", + "cascadeParentIds": [], + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "isInstant": False, + }, + "NATIVE_FILTER-JeZ9HYoTP": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-JeZ9HYoTP", + "isInstant": False, + "name": "Platform", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "platform"}, "datasetId": 33}], + }, + "NATIVE_FILTER-B2PFYVIUw": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-B2PFYVIUw", + "isInstant": False, + "name": "Genre", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "genre"}, "datasetId": 33}], + }, + "NATIVE_FILTER-VDLd4Wq-v": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-VDLd4Wq-v", + "isInstant": False, + "name": "Publisher", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "publisher"}, "datasetId": 33}], + }, + }, + "dataMask": { + "nativeFilters": { + "NATIVE_FILTER-tx05Ze2Hm": { + "extraFormData": {"override_form_data": {"time_range": "No filter"}}, + "currentState": {"value": "No filter"}, + "id": "NATIVE_FILTER-tx05Ze2Hm", + }, + "NATIVE_FILTER-B2PFYVIUw": { + "extraFormData": { + "append_form_data": { + "filters": [ + { + "col": "genre", + "op": "IN", + "val": ["Adventure", "Fighting", "Misc"], + } + ] + } + }, + "currentState": {"value": ["Adventure", "Fighting", "Misc"]}, + "id": "NATIVE_FILTER-B2PFYVIUw", + }, + "NATIVE_FILTER-VDLd4Wq-v": { + "extraFormData": {"append_form_data": {"filters": []}}, + "currentState": {"value": None}, + "id": "NATIVE_FILTER-VDLd4Wq-v", + }, + "NATIVE_FILTER-JeZ9HYoTP": { + "extraFormData": { + "append_form_data": { + "filters": [ + { + "col": "platform", + "op": "IN", + "val": ["GB", "GBA", "PSV", "DS", "3DS"], + } + ] + } + }, + "currentState": {"value": ["GB", "GBA", "PSV", "DS", "3DS"]}, + "id": "NATIVE_FILTER-JeZ9HYoTP", + }, + } + }, +} + +filter_sets_v2 = { + "name": "New filter set", + "id": "FILTERS_SET-tt_Ovwy95", + "nativeFilters": { + "NATIVE_FILTER-tx05Ze2Hm": { + "id": "NATIVE_FILTER-tx05Ze2Hm", + "name": "Time range", + "filterType": "filter_time", + "targets": [{}], + "defaultValue": "No filter", + "cascadeParentIds": [], + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "isInstant": False, + }, + "NATIVE_FILTER-JeZ9HYoTP": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + "defaultToFirstItem": False, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-JeZ9HYoTP", + "isInstant": False, + "name": "Platform", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "platform"}, "datasetId": 33}], + }, + "NATIVE_FILTER-B2PFYVIUw": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + "defaultToFirstItem": False, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-B2PFYVIUw", + "isInstant": False, + "name": "Genre", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "genre"}, "datasetId": 33}], + }, + "NATIVE_FILTER-VDLd4Wq-v": { + "cascadeParentIds": [], + "controlValues": { + "enableEmptyFilter": False, + "inverseSelection": False, + "multiSelect": True, + "sortAscending": True, + "defaultToFirstItem": False, + }, + "defaultValue": None, + "filterType": "filter_select", + "id": "NATIVE_FILTER-VDLd4Wq-v", + "isInstant": False, + "name": "Publisher", + "scope": {"excluded": [], "rootPath": ["ROOT_ID"]}, + "targets": [{"column": {"name": "publisher"}, "datasetId": 33}], + }, + }, + "dataMask": { + "NATIVE_FILTER-tx05Ze2Hm": { + "id": "NATIVE_FILTER-tx05Ze2Hm", + "filterState": {"value": "No filter"}, + "extraFormData": {"time_range": "No filter"}, + }, + "NATIVE_FILTER-B2PFYVIUw": { + "id": "NATIVE_FILTER-B2PFYVIUw", + "filterState": {"value": ["Adventure", "Fighting", "Misc"]}, + "extraFormData": { + "filters": [ + { + "col": "genre", + "op": "IN", + "val": ["Adventure", "Fighting", "Misc"], + } + ] + }, + }, + "NATIVE_FILTER-VDLd4Wq-v": { + "id": "NATIVE_FILTER-VDLd4Wq-v", + "filterState": {"value": None}, + "extraFormData": {"filters": []}, + }, + "NATIVE_FILTER-JeZ9HYoTP": { + "id": "NATIVE_FILTER-JeZ9HYoTP", + "filterState": {"value": ["GB", "GBA", "PSV", "DS", "3DS"]}, + "extraFormData": { + "filters": [ + { + "col": "platform", + "op": "IN", + "val": ["GB", "GBA", "PSV", "DS", "3DS"], + } + ] + }, + }, + }, +} + + +def test_upgrade_select_filters(): + """ + ensure that controlValue.defaultToFirstItem is added if it's missing + """ + converted_filters = deepcopy(native_filters_v1) + upgrade_select_filters(converted_filters) + assert converted_filters == native_filters_v2 + + +def test_upgrade_filter_sets(): + """ + ensure that filter set upgrade operation produces a object that is compatible + with a currently functioning set + """ + converted_filter_set = deepcopy(filter_sets_v1) + upgrade_filter_set(converted_filter_set) + assert converted_filter_set == filter_sets_v2 + + +def test_downgrade_filter_set(): + """ + ensure that the filter set downgrade operation produces an almost identical dict + as the original value + """ + converted_v1_set = deepcopy(filter_sets_v1) + # upgrade the native filter metadata in the comparison fixture, + # as removing the defaultToFirstItem is not necessary + upgrade_select_filters(converted_v1_set["nativeFilters"].values()) + + converted_filter_set = deepcopy(filter_sets_v2) + downgrade_filter_set(converted_filter_set) + assert converted_filter_set == converted_v1_set